On Tue 23 Jun 2009 at 01:34PM, [email protected] wrote:
> Folks,
> For a while, our transport system has not really been adequate to
> address all of the goals for the pkg project. The python libraries that
> we have been using are capabable for basic functionality, but fall flat
> when we try to perform more complicated or performant network
> operations.
>
> The following webrev converts our transport from using the
> urllib/httplib in python to PyCurl, which is based upon libcurl.
>
> http://cr.opensolaris.org/~johansen/webrev-xport-1/
>
> Comments welcome.
Here is my initial batch...
--
Meta-comments/questions
Top level comment: This looks very good.
Now that 'nasty' is (going) in the gate, would it make sense to have
a "stress" section in the test suite which somehow used nasty mode?
Also-- I could run ipkg.sfbay/nasty, a front for dev/ with a different,
nasty personality.
--
client.py:2443 -- would be really nice if this snippet could
move into the transport layer (or somewhere common). I noted
that it is repeated in the GUI. Probably doesn't exist
at all in the update manager...
--
fileobj.py: I'm not sure the comment at 223 is right-- it
talks about curl, but this object seems to not know
anything about curl.
Also, I didn't really understand the header processing,
as I mentioned to you offline...
--
pkgplan.py: I don't totally get this programming model--
why would mfile be None after the call to multi_file? And
wouldn't we want that to be an error of some sort? Or,
should it be something like:
mfile = self.image.transport.multi_file(...)
if mfile is not None:
do stuff
self.__progtrack.download_end_pkg()
?
--
modules/client/transport/stats.py:
Perhaps dump should just wrap around __str__() for this object,
which would format stuff as you like? Might be more flexible
for future expansion.
--
modules/server/catalog.py:
You might want to add a comment that we're rolling the dice
twice here-- first, to see if we're going to be nasty at all,
and then again later on specific lines.
--
modules/server/repository.py:
Not sure how NastyRepository:catalog is different from its
superclass's implementation. I might have missed it somehow?
--
SUNWpython-pycurl/Makefile:
31-32-- I know you copied this from somewhere, but please
nuke this comment.
--
setup.py: why is PCHASH none?
--
t_api_search.py: I take it 'sleep(1)' is to ensure unique
timestamps? Maybe we should have pkgsend_bulk() keep some
state around so that it can avoid this as needed.
--
-dp
--
Daniel Price, Solaris Kernel Engineering http://blogs.sun.com/dp
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss