On Mon, Apr 13, 2009 at 02:47:42PM -0700, Brock Pytlik wrote: > [email protected] wrote: >>> Webrev: >>> http://cr.opensolaris.org/~bpytlik/ips-7367-v1/ >>> >> >> api.py: >> >> I'm concerned that you're not catching all of the different exceptions >> that can come out of the current transport code. If I were you, I'd >> move remote_search to retrieve.py, and have the api call become a >> wrapper for retrieve.remote_search. >> > I'm fine with moving the function wholesale into retrieve and making the > call in API.py a pure wrapper. I'd rather not refactor at this stage > unless absolutely necessary.
That seems reasonable. I'd like to see where the webrev leaves us before saying that I like that idea for sure. >> With that method moved into retrieve, it would be a lot easier for you >> to compare against the different methods and see what error handling is >> missing from the current code. From a quick look, you still need to >> catch retryable HTTP errors, socket timeouts that have been wrapped in a >> URLError, retryable socket errors, ValueError and >> httplib.IncompleteRead. If you're able to determine the content-length >> of a search response, you should also probably be checking that, too. >> > I think I'm only missing ValueError and httplib.IncompleteRead. > It's streamed so I don't think checking content length is possible, > though please correct me if I'm wrong. I was able to play some games to get this to work for catalog streams, but it sounds like the search data is truly generated dynamically, so it probably wouldn't work. If you're not making search/1 a retryable transport method, then you probably don't have to catch the retryable errors. >> The code that's reading from a fileobject also must catch ValueError and >> httplib.IncompleteRead, and retryable socket errors. I'd encourage you >> to take another look at the error handling code in retrieve.py. >> Get_versions() or get_manifest() are both good examples. >> > Ok, I'll expand this bug then to cover the much larger number of > possible error cases. Thanks. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
