On 07/07/2016 10:22 PM, Alex Rousskov wrote:
On 07/06/2016 10:52 PM, Amos Jeffries wrote:
On 7/07/2016 10:24 a.m., Alex Rousskov wrote:
Q2. Where does the pending Downloader class belong?

In overview I think if we have a good Downloader design those other
things and ESIInclude should probably be converted into Downloader or
children of it.

Yes, but if and only if ESI needs to download small SBuf-storable
things. If ESI needs potentially large HTTP response bodies, then ESI
cannot be refactored to use Downloader.


My sense is that the ESI uses simple buffers to store data. However not sure, we need to review the esi code. But the Downloader as is now is a small class, can be extended in the future to handle large response bodies.

If I am not wrong we fixed recently a bug in esi which was related. Maybe there are still others bugs in esi too. Using a class like the Downloader, which used by other subsystems where there is active development and higher users base, may eliminate this type of bugs.


On 07/08/2016 01:16 AM, Amos Jeffries wrote:
On 8/07/2016 7:22 a.m., Alex Rousskov wrote:
   2. Move that remaining generic code into src/servers/Server and src/http/.

We all have been doing step #1 since then, but we are not yet ready for
step #2 because ConnStateData still has HTTP server-specific code in it.

Apart from auditing all I'm seeing from Factory is more and more
non-Server things being added to ConnStateData. First it was 1xx message
handling, then ssl-bump, then all the FTP translation logics, then
Downloader (thanks for reversing on that one).
The 1xx message handling is used by FTP code too.

The ssl-bump my opinion is that it is normal to be handled by ConnStateData because it is near the connection level, not near the protocol level. We may want to split ConnStateData to a new TlsConnStateData class.

There is not a lot of Ftp related code inside ConnStateData. In the future we can completely remove the remaining "if (HTTP protocol)" or "if (FTP protocol)" which are not many.

Also we may want to move the ProxyProtocol related code under a new class inside servers/ProxyProtocol.[cc,h].


_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to