Add support for media uploads from discovery information.

Add support for media uploads from discovery information.

[aiuto, 2011-06-27 19:37:04]

Still have to look at mimeparse, but it seems any comments there are independent
of these.

http://codereview.appspot.com/4664050/diff/1/apiclient/discovery.py
File apiclient/discovery.py (right):

http://codereview.appspot.com/4664050/diff/1/apiclient/discovery.py#newcode232
apiclient/discovery.py:232: MULTIPLIERS = {
I would love to have this block in a common place where I could use it in the
code generator. It's not much, but if we could unify more of the code for
discovery parsing it would be a win.

So, no problem with this block, but maybe something we can do in the future.

http://codereview.appspot.com/4664050/diff/1/apiclient/discovery.py#newcode296
apiclient/discovery.py:296: methodDesc['parameters']['body']['required'] = False
Is this supposed to be indented? It looks like it is alwasy setting to false,
when you could have just set it False at lint 288.

http://codereview.appspot.com/4664050/diff/1/apiclient/errors.py
File apiclient/errors.py (right):

http://codereview.appspot.com/4664050/diff/1/apiclient/errors.py#newcode75
apiclient/errors.py:75: """That is an unacceptable mimetype for this
operation."""
The wording seems slightly off for an error message, but I don't have any better
suggestions. Leave as is if you like it.

[aiuto, 2011-06-27 19:58:30]

mimeparse needs a lot of formatting work.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py
File apiclient/mimeparse.py (right):

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode1
apiclient/mimeparse.py:1: """MIME-Type Parser
Copyright block?

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode12
apiclient/mimeparse.py:12: - quality():           Determines the quality ('q')
of a mime-type when compared against a list of media-ranges.
Line length in this block

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode22
apiclient/mimeparse.py:22: def parse_mime_type(mime_type):
4 char indent? The rest of the library is 2 char.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode23
apiclient/mimeparse.py:23: """Carves up a mime-type and returns a tuple of the
Can you make this a standard PyDoc.
  """One line comment.

  Long desc.
  """

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode26
apiclient/mimeparse.py:26: For example, the media range
'application/xhtml;q=0.5' would
Can you arrange for ease of reading before after, like

the media range: xxxx
would parse to:  yyyyy

OTOH, it's all moot if PyDoc will reorganize the lines.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode31
apiclient/mimeparse.py:31: parts = mime_type.split(";")
Inconsistent use of " and ' through the module.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode33
apiclient/mimeparse.py:33: for param in parts[1:] ])
Not sure of the line wrap. Maybe something that shows the
nesting better
    dict([tuple([s.strip for s in param.split)])
          for param in parts[1:]
         ])
Ahhh. I don't know. It's a subtle line

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode74
apiclient/mimeparse.py:74: if (type == target_type or type == '*' or target_type
== '*') and \
I think it's more readable with '(subtype ..' directly
under '(type'

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode76
apiclient/mimeparse.py:76: param_matches = reduce(lambda x, y: x+y, [1 for (key,
value) in \
By brain blew up in the middle of this. I eventually saw what was happening, but
I would be very scared if I wanted to edit it.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode89
apiclient/mimeparse.py:89: """Find the best match for a given mime-type against
re-indent for line size.

[jcgregorio_google, 2011-06-28 18:42:02]

http://codereview.appspot.com/4664050/diff/1/apiclient/discovery.py
File apiclient/discovery.py (right):

http://codereview.appspot.com/4664050/diff/1/apiclient/discovery.py#newcode232
apiclient/discovery.py:232: MULTIPLIERS = {
Is there any reason you can't include all of apiclient?

On 2011/06/27 19:37:04, aiuto wrote:
> I would love to have this block in a common place where I could use it in the
> code generator. It's not much, but if we could unify more of the code for
> discovery parsing it would be a win.
> 
> So, no problem with this block, but maybe something we can do in the future.

http://codereview.appspot.com/4664050/diff/1/apiclient/discovery.py#newcode296
apiclient/discovery.py:296: methodDesc['parameters']['body']['required'] = False
Fixed

On 2011/06/27 19:37:04, aiuto wrote:
> Is this supposed to be indented? It looks like it is alwasy setting to false,
> when you could have just set it False at lint 288.

http://codereview.appspot.com/4664050/diff/1/apiclient/errors.py
File apiclient/errors.py (right):

http://codereview.appspot.com/4664050/diff/1/apiclient/errors.py#newcode75
apiclient/errors.py:75: """That is an unacceptable mimetype for this
operation."""
Only thing I can think of is s/unacceptable/incorrect/.

On 2011/06/27 19:37:04, aiuto wrote:
> The wording seems slightly off for an error message, but I don't have any
better
> suggestions. Leave as is if you like it.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py
File apiclient/mimeparse.py (right):

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode1
apiclient/mimeparse.py:1: """MIME-Type Parser
On 2011/06/27 19:58:30, aiuto wrote:
> Copyright block?

Done.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode12
apiclient/mimeparse.py:12: - quality():           Determines the quality ('q')
of a mime-type when compared against a list of media-ranges.
On 2011/06/27 19:58:30, aiuto wrote:
> Line length in this block

Done.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode22
apiclient/mimeparse.py:22: def parse_mime_type(mime_type):
This is a "third party" library just copied here to avoid yet another
dependency. I put "third party" in quotes because I wrote it. All the
suggestions are good so I'm doing them and floating them back to the source lib.

 http://code.google.com/p/mimeparse/



On 2011/06/27 19:58:30, aiuto wrote:
> 4 char indent? The rest of the library is 2 char.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode23
apiclient/mimeparse.py:23: """Carves up a mime-type and returns a tuple of the
On 2011/06/27 19:58:30, aiuto wrote:
> Can you make this a standard PyDoc.
>   """One line comment.
> 
>   Long desc.
>   """

Done.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode31
apiclient/mimeparse.py:31: parts = mime_type.split(";")
On 2011/06/27 19:58:30, aiuto wrote:
> Inconsistent use of " and ' through the module.

Done.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode33
apiclient/mimeparse.py:33: for param in parts[1:] ])
On 2011/06/27 19:58:30, aiuto wrote:
> Not sure of the line wrap. Maybe something that shows the
> nesting better
>     dict([tuple([s.strip for s in param.split)])
>           for param in parts[1:]
>          ])
> Ahhh. I don't know. It's a subtle line

Done.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode74
apiclient/mimeparse.py:74: if (type == target_type or type == '*' or target_type
== '*') and \
On 2011/06/27 19:58:30, aiuto wrote:
> I think it's more readable with '(subtype ..' directly
> under '(type'             

Done.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode76
apiclient/mimeparse.py:76: param_matches = reduce(lambda x, y: x+y, [1 for (key,
value) in \
Same here, which is why I don't want to change it.

On 2011/06/27 19:58:30, aiuto wrote:
> By brain blew up in the middle of this. I eventually saw what was happening,
but
> I would be very scared if I wanted to edit it.

http://codereview.appspot.com/4664050/diff/1/apiclient/mimeparse.py#newcode89
apiclient/mimeparse.py:89: """Find the best match for a given mime-type against
On 2011/06/27 19:58:30, aiuto wrote:
> re-indent for line size.

Done.

[aiuto, 2011-06-29 02:39:37]

lgtm

just a few nits.

AND....  somehow this still cc's
google-api-python-client@googlegroups.com

which does not exist.  Who can fix that?

http://codereview.appspot.com/4664050/diff/7006/apiclient/discovery.py
File apiclient/discovery.py (right):

http://codereview.appspot.com/4664050/diff/7006/apiclient/discovery.py#newcod...
apiclient/discovery.py:387: if not mimeparse.best_match([media_mime_type],
",".join(accept)):
' rather than "

http://codereview.appspot.com/4664050/diff/7006/apiclient/errors.py
File apiclient/errors.py (right):

http://codereview.appspot.com/4664050/diff/7006/apiclient/errors.py#newcode44
apiclient/errors.py:44: """
""" on the previous line or you'll get a lint failure when you sync.

http://codereview.appspot.com/4664050/diff/7006/apiclient/mimeparse.py
File apiclient/mimeparse.py (right):

http://codereview.appspot.com/4664050/diff/7006/apiclient/mimeparse.py#newcod...
apiclient/mimeparse.py:120: parsed media ranges. """
""" on the next line

[jcgregorio_google, 2011-06-29 16:06:48]

Ah, google-api-python-client@googlegroups.com is a valid email address, but you
have to join the google group because the group doesn't allow non-member emails.

http://codereview.appspot.com/4664050/diff/7006/apiclient/discovery.py
File apiclient/discovery.py (right):

http://codereview.appspot.com/4664050/diff/7006/apiclient/discovery.py#newcod...
apiclient/discovery.py:387: if not mimeparse.best_match([media_mime_type],
",".join(accept)):
On 2011/06/29 02:39:37, aiuto wrote:
> ' rather than "

Done.

http://codereview.appspot.com/4664050/diff/7006/apiclient/errors.py
File apiclient/errors.py (right):

http://codereview.appspot.com/4664050/diff/7006/apiclient/errors.py#newcode44
apiclient/errors.py:44: """
On 2011/06/29 02:39:37, aiuto wrote:
> """ on the previous line or you'll get a lint failure when you sync.

Done.

http://codereview.appspot.com/4664050/diff/7006/apiclient/mimeparse.py
File apiclient/mimeparse.py (right):

http://codereview.appspot.com/4664050/diff/7006/apiclient/mimeparse.py#newcod...
apiclient/mimeparse.py:120: parsed media ranges. """
On 2011/06/29 02:39:37, aiuto wrote:
> """ on the next line

Done.

[aiuto, 2011-06-30 02:19:08]

LGTM