Opened 11 months ago

Closed 9 months ago

#5099 closed enhancement (complete)

Create HttpListener, HttpConnection and HttpConnectionPool in libkea-http

Reported by: marcin Owned by: marcin
Priority: medium Milestone: Kea1.2
Component: remote-management Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

The following section of the Control API design ​http://kea.isc.org/wiki/ControlAPIDesign#ProposedArchitecture calls for creating the HttpConnection, HttpConnectionPool and HttpListener classes which receive new TCP connections, receive HTTP messages and pass them to the HTTP parsers and then send back HTTP responses.

This ticket is about implementing these classes.

Subtickets

Change History (7)

comment:1 Changed 11 months ago by marcin

  • Owner set to marcin
  • Status changed from new to accepted

comment:2 Changed 10 months ago by marcin

  • Owner changed from marcin to UnAssigned
  • Status changed from accepted to reviewing

The ticket is now ready for review.
Proposed ChangeLog entry:

12XX.   [func]         marcin
	Implemented libkea-http library.
        (Trac #5077, git cd72284b5b221e620770883db7e166c4d3ba7eb6)
        (Trac #5088, git 715d18f961801ffbd798a65b19459178c3a53857)
        (Trac #5099, git abcd)

comment:3 Changed 10 months ago by fdupont

  • Owner changed from UnAssigned to fdupont

comment:4 Changed 10 months ago by fdupont

  • Owner changed from fdupont to marcin

Fixed a few spelling errors so please pull.

I believe you use boost::enable_shared_from_this because there was no better choice.

For the std::array I understand it is more efficient for its particular use than a std::vector.

For connection pools IMHO std::list is far better (linear vs logarithmic) than std::set so either change or explain why you have to use a set.

IMHO the reason for virtual ~HttpResponseCreatorFactory() is because it is required for an abstract class, not to help derived class...

There should be a comment about the 64kb limit of TCPSocket<C>::asyncSend: I don't like this at all even I can live with it for now. BTW the Boost Asio documentation of async_send recommends to use async_write instead when complete send is important...

I recommend to check with Jenkins on VMs which raised spurious errors with the TCP acceptor code.

At the exception of the set vs list concern I can't see something against merging this ticket (but I am afraid we'll discover more when we'll try to use this new feature...).

comment:5 Changed 9 months ago by marcin

  • Owner changed from marcin to fdupont

Francis, thanks for your updates and review. I believe I have now addressed your review comments, including the point about the set vs list.

comment:6 Changed 9 months ago by fdupont

  • Owner changed from fdupont to marcin

I have still some concerns (e.g. the 64kB limit) but they are clearly about the design itself... Please merge.

comment:7 Changed 9 months ago by marcin

  • Resolution set to complete
  • Status changed from reviewing to closed

Merged with commit 7e8df7993f295431e2cb6a13858f746649c4e18d

Note: See TracTickets for help on using tickets.