archive.checkUpload() throws exception rather than returning False

[Stefano Rivera]

archive.checkUpload() throws an HTTP exception rather than returning False. This is presumably by design, but painful for launchpadlib use. Having a function with a boolean response is preferable to having to use try...except.

isSourceUploadAllowed() returns boolean, but only for explicit permissions, not for general component-wide permissions (even though its name implies general permission)

[Julian Edwards (julian-edwards)]

checkUpload() raises an exception so that you can have more info about why the upload is not allowed. This is more useful than a simple "False".

Gary, can it doing anything different to be friendlier to the webservice?

[Gary Poster (gary)]

That sounds like a fair reason as to why an exception was used, even simply from a Pythonic perspective, irrespective of webservice involvement. Is the webservice allowing the error message through? We do have a mechanism to allow that.

The only other thought I have is perhaps exporting two APIs, ``checkUpload`` with the current behavior and ``canUpload`` with the requested behavior. That said, someone annoyed by the ``checkUpload`` behavior could simply wrap it with a helper function for what I proposed for the ``canUpload`` behavior, and move on. If I had to choose one of those two behaviors from the webservice itself, I'd choose the current one.

I'll ask Leonard and/or Benji to weigh in, in case they have any other ideas.

[Stefano Rivera (stefanor)]

> The only other thought I have is perhaps exporting two APIs,
> ``checkUpload`` with the current behavior and ``canUpload`` with the
> requested behavior. That said, someone annoyed by the ``checkUpload``
> behavior could simply wrap it with a helper function for what I proposed
> for the ``canUpload`` behavior, and move on.

That's what I was expecting (and what we already doing in
ubuntu-dev-tools, which has all sorts of launchpadlib wrapping)

Yeah, the only issues with it are that it's not Pythonic, and that the
documentation says nothing about throwing exceptions instead of
returning something.

SR

--
Stefano Rivera
  http://tumbleweed.org.za/
  H: [phone] C: [phone] UCT: x3127

[Leonard Richardson (leonardr)]

checkUpload/canUpload would be fine. The current behavior is also fine, if documented properly. (quality of +apidoc documentation is definitely a huge shortcoming for us right now)

There's a network boundary between the client and the server, so there's always going to be some un-Pythonic behavior. We've tried to minimize this by mapping 2xx-3xx error codes onto normal Python object operations, and 4xx-5xx error codes onto Python exceptions, but some stuff will still not make sense.

[Julian Edwards (julian-edwards)]

On Wednesday 27 October 2010 15:48:53 you wrote:
> > The only other thought I have is perhaps exporting two APIs,
> > ``checkUpload`` with the current behavior and ``canUpload`` with the
> > requested behavior. That said, someone annoyed by the ``checkUpload``
> > behavior could simply wrap it with a helper function for what I proposed
> > for the ``canUpload`` behavior, and move on.
>
> That's what I was expecting (and what we already doing in
> ubuntu-dev-tools, which has all sorts of launchpadlib wrapping)
>
> Yeah, the only issues with it are that it's not Pythonic, and that the
> documentation says nothing about throwing exceptions instead of
> returning something.

Right, the documentation is bad, I will update that.

The checkUpload() itself is wrapping something that doesn't raise exceptions.
We can expose that to return True or False if it helps.