[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.
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.
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.
Brock
-j


_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to