On Fri, Jan 08, 2010 at 02:45:50PM -0600, Shawn Walker wrote: > On 01/ 8/10 02:30 PM, [email protected] wrote: > >Folks, > >The following webrev fixes a pair of transport issues. One allows the > >client to use SSL/TLS without providing a key/cert. The other has the > >client shutdown open connections after the download phase to conserve > >server-side resources. (This helps the test suite go faster too). > > > > http://cr.opensolaris.org/~johansen/webrev-13746/ > > Only comment I have is that it seems like the self.__engine, > self.__setup() check and call could be moved into > _captive_portal_test to eliminate explicitly having to call it in > many cases. Would that be wrong?
It's like an english paper: there's no wrong answers. I considered that approach, but thought it made more sense to expilictly add the setup check to each client operation. The captive portal test calls get_versions(), so it seemed like it made more sense for the check to be in get_versions instead. In particular, this code continues to work if we ever remove the captive_portal test. I recall some thread long ago where you expressed a desire to eventually excise this test from the code entirely. > I keep thinking that the transport should be shutdown in some other > case too. Did you have any specific case in mind? When the client is done with the download, we're almost certainly done unless an informational operation is requested later by the GUI. > How long will the connections stay open/idle? I'm primarily > thinking of the pkg list / pkg info cases in the GUI. Keep in mind that with pipelining, we only keep one connection open per host as it is. Libcurl gets to decide how long it caches connections. That's not tunable by the libary's interface. For informational operations, chances are that you'll open only one (or re-use an existing connection). Remember that the problem we saw in the test suite was the consequence of generating lots of Transport objects, each with one connection, that weren't garbage collected during the test run. This is a bit of a corner case, IMO. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
