Issue 6610049: Moving Resource class to module level to make it serializable.

dhermes

Sorry this just got sent. I am still not 100% sure what causes rietveld to ...

11 years, 10 months ago (2012-10-16 03:59:34 UTC) #2

dhermes

Hey Joe, Now that you're back, will you have a chance to review this? On ...

11 years, 10 months ago (2012-10-25 02:38:43 UTC) #3

jcgregorio_google

https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py File tests/test_discovery.py (right): https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py#newcode805 tests/test_discovery.py:805: # http = HttpMock(datafile('plus.json'), {'status': '200'}) Leftover commented out ...

11 years, 10 months ago (2012-10-25 18:41:59 UTC) #4

dhermes

https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py File tests/test_discovery.py (right): https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py#newcode805 tests/test_discovery.py:805: # http = HttpMock(datafile('plus.json'), {'status': '200'}) On 2012/10/25 18:41:59, ...

11 years, 10 months ago (2012-10-25 18:43:46 UTC) #5

dhermes

Any progress on this review? On Thu, Oct 25, 2012 at 11:43 AM, <dhermes@google.com> wrote: ...

11 years, 10 months ago (2012-10-29 20:56:47 UTC) #6

jcgregorio_google

https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py File apiclient/discovery.py (right): https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newcode384 apiclient/discovery.py:384: one blank line https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newcode634 apiclient/discovery.py:634: previous_response: The response from ...

11 years, 10 months ago (2012-10-30 18:03:04 UTC) #7

dhermes

I have already updated the new patch set. https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py File apiclient/discovery.py (right): https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newcode384 apiclient/discovery.py:384: On ...

11 years, 10 months ago (2012-10-30 18:12:19 UTC) #8

jcgregorio_google

Have you also verified that this doesn't change the output of 'make docs' ? -joe ...

11 years, 10 months ago (2012-10-30 18:17:01 UTC) #9

dhermes

Nope. How might one do this? On Tue, Oct 30, 2012 at 11:17 AM, Joe ...

11 years, 10 months ago (2012-10-30 18:35:12 UTC) #10

jcgregorio_google

There's a Makefile in the project. Just run: $ make docs Then look at the ...

11 years, 10 months ago (2012-10-30 18:37:50 UTC) #11

There's a Makefile in the project. Just run:

$ make docs

Then look at the diffs in the docs that produces. You probably want to
patch this CL into
another copy of the repository since it will modify the doc files
which you probably don't want
as a part of this CL.

  Thanks,
  -joe

On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> wrote:
> Nope. How might one do this?
>
>
> On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com>
> wrote:
>>
>> Have you also verified that this doesn't change the output of 'make docs'
>> ?
>>
>>   -joe
>>
>> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
>> > I have already updated the new patch set.
>> >
>> >
>> >
>> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
>> > File apiclient/discovery.py (right):
>> >
>> >
>> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> > apiclient/discovery.py:384:
>> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >>
>> >> one blank line
>> >
>> >
>> > Done.
>> >
>> >
>> >
>> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> > apiclient/discovery.py:634: previous_response: The response from the
>> > request for the previous page. (required)
>> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >>
>> >> 80 chars
>> >
>> >
>> > Done.
>> >
>> >
>> >
>> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> > apiclient/discovery.py:670: def __init__(self, http=None, baseUrl=None,
>> > model=None, developerKey=None,
>> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >>
>> >> I don't think any of these needs =None defaults.
>> >
>> >
>> > You are right. I'm not sure why I changed this, and also accidentally
>> > changed the order of developerKey and requestBuilder.
>> >
>> >
>> >
>> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> > apiclient/discovery.py:739: def set_service_methods(self):
>> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >>
>> >> _set_service_methods
>> >
>> >
>> > Done.
>> >
>> > https://codereview.appspot.com/6610049/
>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer

Sign in to reply to this message.

dhermes

It does change things. For example it is removing all the subresources from specific API ...

11 years, 10 months ago (2012-10-30 19:34:04 UTC) #12

It does change things. For example it is removing all the subresources from
specific API docs.

This is likely because the attr is not set on the object so object.__dict__
does not get updated and things like dir(service_object) won't list the
subresources.

I can set this in a method that will also set it in _attr_cache or just
track the attributes that would have ended up in attr_cache via some other
mechanism.

What do you think?


On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com>wrote:

> There's a Makefile in the project. Just run:
>
> $ make docs
>
> Then look at the diffs in the docs that produces. You probably want to
> patch this CL into
> another copy of the repository since it will modify the doc files
> which you probably don't want
> as a part of this CL.
>
>   Thanks,
>   -joe
>
> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> wrote:
> > Nope. How might one do this?
> >
> >
> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com>
> > wrote:
> >>
> >> Have you also verified that this doesn't change the output of 'make
> docs'
> >> ?
> >>
> >>   -joe
> >>
> >> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
> >> > I have already updated the new patch set.
> >> >
> >> >
> >> >
> >> >
> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
> >> > File apiclient/discovery.py (right):
> >> >
> >> >
> >> >
>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
> >> > apiclient/discovery.py:384:
> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
> >> >>
> >> >> one blank line
> >> >
> >> >
> >> > Done.
> >> >
> >> >
> >> >
> >> >
>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
> >> > apiclient/discovery.py:634: previous_response: The response from the
> >> > request for the previous page. (required)
> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
> >> >>
> >> >> 80 chars
> >> >
> >> >
> >> > Done.
> >> >
> >> >
> >> >
> >> >
>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
> >> > apiclient/discovery.py:670: def __init__(self, http=None,
> baseUrl=None,
> >> > model=None, developerKey=None,
> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
> >> >>
> >> >> I don't think any of these needs =None defaults.
> >> >
> >> >
> >> > You are right. I'm not sure why I changed this, and also accidentally
> >> > changed the order of developerKey and requestBuilder.
> >> >
> >> >
> >> >
> >> >
>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
> >> > apiclient/discovery.py:739: def set_service_methods(self):
> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
> >> >>
> >> >> _set_service_methods
> >> >
> >> >
> >> > Done.
> >> >
> >> > https://codereview.appspot.com/6610049/
> >
> >
> >
> >
> > --
> > Danny Hermes
> > Developer Programs Engineer
>



-- 
Danny Hermes
Developer Programs Engineer

Sign in to reply to this message.

jcgregorio_google

On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote: > It does ...

11 years, 10 months ago (2012-10-30 19:43:37 UTC) #13

On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote:
> It does change things. For example it is removing all the subresources from
> specific API docs.
>
> This is likely because the attr is not set on the object so object.__dict__
> does not get updated and things like dir(service_object) won't list the
> subresources.

Yeah, definitely want to keep the dir(service_object) behavior unchanged.

> I can set this in a method that will also set it in _attr_cache or just
> track the attributes that would have ended up in attr_cache via some other
> mechanism.

Or would it be easier to invert the meaning of attr_cache and have it
just be the name of attributes that will be stripped out of the response from
__getstate__? That way you can also avoid over-riding __getattribute__.

 -joe

>
> What do you think?
>
>
> On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com>
> wrote:
>>
>> There's a Makefile in the project. Just run:
>>
>> $ make docs
>>
>> Then look at the diffs in the docs that produces. You probably want to
>> patch this CL into
>> another copy of the repository since it will modify the doc files
>> which you probably don't want
>> as a part of this CL.
>>
>>   Thanks,
>>   -joe
>>
>> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> wrote:
>> > Nope. How might one do this?
>> >
>> >
>> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com>
>> > wrote:
>> >>
>> >> Have you also verified that this doesn't change the output of 'make
>> >> docs'
>> >> ?
>> >>
>> >>   -joe
>> >>
>> >> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
>> >> > I have already updated the new patch set.
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
>> >> > File apiclient/discovery.py (right):
>> >> >
>> >> >
>> >> >
>> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> >> > apiclient/discovery.py:384:
>> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >> >>
>> >> >> one blank line
>> >> >
>> >> >
>> >> > Done.
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> >> > apiclient/discovery.py:634: previous_response: The response from the
>> >> > request for the previous page. (required)
>> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >> >>
>> >> >> 80 chars
>> >> >
>> >> >
>> >> > Done.
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> >> > apiclient/discovery.py:670: def __init__(self, http=None,
>> >> > baseUrl=None,
>> >> > model=None, developerKey=None,
>> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >> >>
>> >> >> I don't think any of these needs =None defaults.
>> >> >
>> >> >
>> >> > You are right. I'm not sure why I changed this, and also accidentally
>> >> > changed the order of developerKey and requestBuilder.
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> >> > apiclient/discovery.py:739: def set_service_methods(self):
>> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >> >>
>> >> >> _set_service_methods
>> >> >
>> >> >
>> >> > Done.
>> >> >
>> >> > https://codereview.appspot.com/6610049/
>> >
>> >
>> >
>> >
>> > --
>> > Danny Hermes
>> > Developer Programs Engineer
>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer

Sign in to reply to this message.

dhermes

Yes. That is what I meant. Will update with a patch. On Tue, Oct 30, ...

11 years, 10 months ago (2012-10-30 19:46:54 UTC) #14

Yes. That is what I meant. Will update with a patch.


On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>wrote:

> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote:
> > It does change things. For example it is removing all the subresources
> from
> > specific API docs.
> >
> > This is likely because the attr is not set on the object so
> object.__dict__
> > does not get updated and things like dir(service_object) won't list the
> > subresources.
>
> Yeah, definitely want to keep the dir(service_object) behavior unchanged.
>
> > I can set this in a method that will also set it in _attr_cache or just
> > track the attributes that would have ended up in attr_cache via some
> other
> > mechanism.
>
> Or would it be easier to invert the meaning of attr_cache and have it
> just be the name of attributes that will be stripped out of the response
> from
> __getstate__? That way you can also avoid over-riding __getattribute__.
>
>  -joe
>
> >
> > What do you think?
> >
> >
> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com>
> > wrote:
> >>
> >> There's a Makefile in the project. Just run:
> >>
> >> $ make docs
> >>
> >> Then look at the diffs in the docs that produces. You probably want to
> >> patch this CL into
> >> another copy of the repository since it will modify the doc files
> >> which you probably don't want
> >> as a part of this CL.
> >>
> >>   Thanks,
> >>   -joe
> >>
> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com>
> wrote:
> >> > Nope. How might one do this?
> >> >
> >> >
> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com
> >
> >> > wrote:
> >> >>
> >> >> Have you also verified that this doesn't change the output of 'make
> >> >> docs'
> >> >> ?
> >> >>
> >> >>   -joe
> >> >>
> >> >> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
> >> >> > I have already updated the new patch set.
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
> >> >> > File apiclient/discovery.py (right):
> >> >> >
> >> >> >
> >> >> >
> >> >> >
>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
> >> >> > apiclient/discovery.py:384:
> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
> >> >> >>
> >> >> >> one blank line
> >> >> >
> >> >> >
> >> >> > Done.
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> >
>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
> >> >> > apiclient/discovery.py:634: previous_response: The response from
> the
> >> >> > request for the previous page. (required)
> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
> >> >> >>
> >> >> >> 80 chars
> >> >> >
> >> >> >
> >> >> > Done.
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> >
>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
> >> >> > apiclient/discovery.py:670: def __init__(self, http=None,
> >> >> > baseUrl=None,
> >> >> > model=None, developerKey=None,
> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
> >> >> >>
> >> >> >> I don't think any of these needs =None defaults.
> >> >> >
> >> >> >
> >> >> > You are right. I'm not sure why I changed this, and also
> accidentally
> >> >> > changed the order of developerKey and requestBuilder.
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> >
>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
> >> >> > apiclient/discovery.py:739: def set_service_methods(self):
> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
> >> >> >>
> >> >> >> _set_service_methods
> >> >> >
> >> >> >
> >> >> > Done.
> >> >> >
> >> >> > https://codereview.appspot.com/6610049/
> >> >
> >> >
> >> >
> >> >
> >> > --
> >> > Danny Hermes
> >> > Developer Programs Engineer
> >
> >
> >
> >
> > --
> > Danny Hermes
> > Developer Programs Engineer
>



-- 
Danny Hermes
Developer Programs Engineer

Sign in to reply to this message.

dhermes

Patch uploaded. There are still some docs that end up different, but they seem to ...

11 years, 10 months ago (2012-10-30 20:26:38 UTC) #15

Patch uploaded. There are still some docs that end up different, but they
seem to have been patched by human hands.

Also, all of a sudden, make docs is failing, complaining about a right tick
mark (\u2019) when it tries to right to a file with no codecs. I checked
and double-checked and this has not been introduced in my code.

Have you guys experienced this?


On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com> wrote:

> Yes. That is what I meant. Will update with a patch.
>
>
> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>wrote:
>
>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote:
>> > It does change things. For example it is removing all the subresources
>> from
>> > specific API docs.
>> >
>> > This is likely because the attr is not set on the object so
>> object.__dict__
>> > does not get updated and things like dir(service_object) won't list the
>> > subresources.
>>
>> Yeah, definitely want to keep the dir(service_object) behavior unchanged.
>>
>> > I can set this in a method that will also set it in _attr_cache or just
>> > track the attributes that would have ended up in attr_cache via some
>> other
>> > mechanism.
>>
>> Or would it be easier to invert the meaning of attr_cache and have it
>> just be the name of attributes that will be stripped out of the response
>> from
>> __getstate__? That way you can also avoid over-riding __getattribute__.
>>
>>  -joe
>>
>> >
>> > What do you think?
>> >
>> >
>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com>
>> > wrote:
>> >>
>> >> There's a Makefile in the project. Just run:
>> >>
>> >> $ make docs
>> >>
>> >> Then look at the diffs in the docs that produces. You probably want to
>> >> patch this CL into
>> >> another copy of the repository since it will modify the doc files
>> >> which you probably don't want
>> >> as a part of this CL.
>> >>
>> >>   Thanks,
>> >>   -joe
>> >>
>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com>
>> wrote:
>> >> > Nope. How might one do this?
>> >> >
>> >> >
>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <
>> jcgregorio@google.com>
>> >> > wrote:
>> >> >>
>> >> >> Have you also verified that this doesn't change the output of 'make
>> >> >> docs'
>> >> >> ?
>> >> >>
>> >> >>   -joe
>> >> >>
>> >> >> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
>> >> >> > I have already updated the new patch set.
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
>> >> >> > File apiclient/discovery.py (right):
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> >> >> > apiclient/discovery.py:384:
>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >> >> >>
>> >> >> >> one blank line
>> >> >> >
>> >> >> >
>> >> >> > Done.
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> >> >> > apiclient/discovery.py:634: previous_response: The response from
>> the
>> >> >> > request for the previous page. (required)
>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >> >> >>
>> >> >> >> 80 chars
>> >> >> >
>> >> >> >
>> >> >> > Done.
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None,
>> >> >> > baseUrl=None,
>> >> >> > model=None, developerKey=None,
>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >> >> >>
>> >> >> >> I don't think any of these needs =None defaults.
>> >> >> >
>> >> >> >
>> >> >> > You are right. I'm not sure why I changed this, and also
>> accidentally
>> >> >> > changed the order of developerKey and requestBuilder.
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >
>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>> >> >> > apiclient/discovery.py:739: def set_service_methods(self):
>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>> >> >> >>
>> >> >> >> _set_service_methods
>> >> >> >
>> >> >> >
>> >> >> > Done.
>> >> >> >
>> >> >> > https://codereview.appspot.com/6610049/
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Danny Hermes
>> >> > Developer Programs Engineer
>> >
>> >
>> >
>> >
>> > --
>> > Danny Hermes
>> > Developer Programs Engineer
>>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer
>



-- 
Danny Hermes
Developer Programs Engineer

Sign in to reply to this message.

jcgregorio_google

Yeah, I thought I'd pushed a fix for that, there was an issue with unicode ...

11 years, 10 months ago (2012-10-30 20:28:49 UTC) #16

Yeah, I thought I'd pushed a fix for that, there was an issue with
unicode in descriptions. I will look into that.

  -joe

On Tue, Oct 30, 2012 at 4:26 PM, Danny Hermes <dhermes@google.com> wrote:
> Patch uploaded. There are still some docs that end up different, but they
> seem to have been patched by human hands.
>
> Also, all of a sudden, make docs is failing, complaining about a right tick
> mark (\u2019) when it tries to right to a file with no codecs. I checked and
> double-checked and this has not been introduced in my code.
>
> Have you guys experienced this?
>
>
> On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com> wrote:
>>
>> Yes. That is what I meant. Will update with a patch.
>>
>>
>> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>
>> wrote:
>>>
>>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote:
>>> > It does change things. For example it is removing all the subresources
>>> > from
>>> > specific API docs.
>>> >
>>> > This is likely because the attr is not set on the object so
>>> > object.__dict__
>>> > does not get updated and things like dir(service_object) won't list the
>>> > subresources.
>>>
>>> Yeah, definitely want to keep the dir(service_object) behavior unchanged.
>>>
>>> > I can set this in a method that will also set it in _attr_cache or just
>>> > track the attributes that would have ended up in attr_cache via some
>>> > other
>>> > mechanism.
>>>
>>> Or would it be easier to invert the meaning of attr_cache and have it
>>> just be the name of attributes that will be stripped out of the response
>>> from
>>> __getstate__? That way you can also avoid over-riding __getattribute__.
>>>
>>>  -joe
>>>
>>> >
>>> > What do you think?
>>> >
>>> >
>>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com>
>>> > wrote:
>>> >>
>>> >> There's a Makefile in the project. Just run:
>>> >>
>>> >> $ make docs
>>> >>
>>> >> Then look at the diffs in the docs that produces. You probably want to
>>> >> patch this CL into
>>> >> another copy of the repository since it will modify the doc files
>>> >> which you probably don't want
>>> >> as a part of this CL.
>>> >>
>>> >>   Thanks,
>>> >>   -joe
>>> >>
>>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com>
>>> >> wrote:
>>> >> > Nope. How might one do this?
>>> >> >
>>> >> >
>>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio
>>> >> > <jcgregorio@google.com>
>>> >> > wrote:
>>> >> >>
>>> >> >> Have you also verified that this doesn't change the output of 'make
>>> >> >> docs'
>>> >> >> ?
>>> >> >>
>>> >> >>   -joe
>>> >> >>
>>> >> >> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
>>> >> >> > I have already updated the new patch set.
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
>>> >> >> > File apiclient/discovery.py (right):
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>> >> >> > apiclient/discovery.py:384:
>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>> >> >> >>
>>> >> >> >> one blank line
>>> >> >> >
>>> >> >> >
>>> >> >> > Done.
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>> >> >> > apiclient/discovery.py:634: previous_response: The response from
>>> >> >> > the
>>> >> >> > request for the previous page. (required)
>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>> >> >> >>
>>> >> >> >> 80 chars
>>> >> >> >
>>> >> >> >
>>> >> >> > Done.
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None,
>>> >> >> > baseUrl=None,
>>> >> >> > model=None, developerKey=None,
>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>> >> >> >>
>>> >> >> >> I don't think any of these needs =None defaults.
>>> >> >> >
>>> >> >> >
>>> >> >> > You are right. I'm not sure why I changed this, and also
>>> >> >> > accidentally
>>> >> >> > changed the order of developerKey and requestBuilder.
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>> >> >> > apiclient/discovery.py:739: def set_service_methods(self):
>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>> >> >> >>
>>> >> >> >> _set_service_methods
>>> >> >> >
>>> >> >> >
>>> >> >> > Done.
>>> >> >> >
>>> >> >> > https://codereview.appspot.com/6610049/
>>> >> >
>>> >> >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > Danny Hermes
>>> >> > Developer Programs Engineer
>>> >
>>> >
>>> >
>>> >
>>> > --
>>> > Danny Hermes
>>> > Developer Programs Engineer
>>
>>
>>
>>
>> --
>> Danny Hermes
>> Developer Programs Engineer
>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer

Sign in to reply to this message.

dhermes

Traceback: python describe.py Traceback (most recent call last): File "describe.py", line 346, in <module> document_api(api['name'], ...

11 years, 10 months ago (2012-10-30 20:29:10 UTC) #17

Traceback:

python describe.py
Traceback (most recent call last):
  File "describe.py", line 346, in <module>
    document_api(api['name'], api['version'])
  File "describe.py", line 335, in document_api
    service, '%s_%s.' % (name, version), discovery, discovery)
  File "describe.py", line 314, in document_collection_recursive
    discovery['resources'].get(dname, {}))
  File "describe.py", line 304, in document_collection_recursive
    f.write(html)
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in
position 5677: ordinal not in range(128)
make: *** [docs] Error 1


On Tue, Oct 30, 2012 at 1:26 PM, Danny Hermes <dhermes@google.com> wrote:

> Patch uploaded. There are still some docs that end up different, but they
> seem to have been patched by human hands.
>
> Also, all of a sudden, make docs is failing, complaining about a right
> tick mark (\u2019) when it tries to right to a file with no codecs. I
> checked and double-checked and this has not been introduced in my code.
>
> Have you guys experienced this?
>
>
> On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com> wrote:
>
>> Yes. That is what I meant. Will update with a patch.
>>
>>
>> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>wrote:
>>
>>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com>
>>> wrote:
>>> > It does change things. For example it is removing all the subresources
>>> from
>>> > specific API docs.
>>> >
>>> > This is likely because the attr is not set on the object so
>>> object.__dict__
>>> > does not get updated and things like dir(service_object) won't list the
>>> > subresources.
>>>
>>> Yeah, definitely want to keep the dir(service_object) behavior unchanged.
>>>
>>> > I can set this in a method that will also set it in _attr_cache or just
>>> > track the attributes that would have ended up in attr_cache via some
>>> other
>>> > mechanism.
>>>
>>> Or would it be easier to invert the meaning of attr_cache and have it
>>> just be the name of attributes that will be stripped out of the response
>>> from
>>> __getstate__? That way you can also avoid over-riding __getattribute__.
>>>
>>>  -joe
>>>
>>> >
>>> > What do you think?
>>> >
>>> >
>>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com>
>>> > wrote:
>>> >>
>>> >> There's a Makefile in the project. Just run:
>>> >>
>>> >> $ make docs
>>> >>
>>> >> Then look at the diffs in the docs that produces. You probably want to
>>> >> patch this CL into
>>> >> another copy of the repository since it will modify the doc files
>>> >> which you probably don't want
>>> >> as a part of this CL.
>>> >>
>>> >>   Thanks,
>>> >>   -joe
>>> >>
>>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com>
>>> wrote:
>>> >> > Nope. How might one do this?
>>> >> >
>>> >> >
>>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <
>>> jcgregorio@google.com>
>>> >> > wrote:
>>> >> >>
>>> >> >> Have you also verified that this doesn't change the output of 'make
>>> >> >> docs'
>>> >> >> ?
>>> >> >>
>>> >> >>   -joe
>>> >> >>
>>> >> >> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
>>> >> >> > I have already updated the new patch set.
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
>>> >> >> > File apiclient/discovery.py (right):
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>> >> >> > apiclient/discovery.py:384:
>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>> >> >> >>
>>> >> >> >> one blank line
>>> >> >> >
>>> >> >> >
>>> >> >> > Done.
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>> >> >> > apiclient/discovery.py:634: previous_response: The response from
>>> the
>>> >> >> > request for the previous page. (required)
>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>> >> >> >>
>>> >> >> >> 80 chars
>>> >> >> >
>>> >> >> >
>>> >> >> > Done.
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None,
>>> >> >> > baseUrl=None,
>>> >> >> > model=None, developerKey=None,
>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>> >> >> >>
>>> >> >> >> I don't think any of these needs =None defaults.
>>> >> >> >
>>> >> >> >
>>> >> >> > You are right. I'm not sure why I changed this, and also
>>> accidentally
>>> >> >> > changed the order of developerKey and requestBuilder.
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>> >> >> > apiclient/discovery.py:739: def set_service_methods(self):
>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>> >> >> >>
>>> >> >> >> _set_service_methods
>>> >> >> >
>>> >> >> >
>>> >> >> > Done.
>>> >> >> >
>>> >> >> > https://codereview.appspot.com/6610049/
>>> >> >
>>> >> >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > Danny Hermes
>>> >> > Developer Programs Engineer
>>> >
>>> >
>>> >
>>> >
>>> > --
>>> > Danny Hermes
>>> > Developer Programs Engineer
>>>
>>
>>
>>
>> --
>> Danny Hermes
>> Developer Programs Engineer
>>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer
>



-- 
Danny Hermes
Developer Programs Engineer

Sign in to reply to this message.

dhermes

Can you try building the docs with a working version of make docs? On Tue, ...

11 years, 10 months ago (2012-10-31 22:30:49 UTC) #18

Can you try building the docs with a working version of make docs?


On Tue, Oct 30, 2012 at 1:28 PM, Danny Hermes <dhermes@google.com> wrote:

> Traceback:
>
> python describe.py
> Traceback (most recent call last):
>   File "describe.py", line 346, in <module>
>     document_api(api['name'], api['version'])
>   File "describe.py", line 335, in document_api
>     service, '%s_%s.' % (name, version), discovery, discovery)
>   File "describe.py", line 314, in document_collection_recursive
>     discovery['resources'].get(dname, {}))
>   File "describe.py", line 304, in document_collection_recursive
>     f.write(html)
> UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in
> position 5677: ordinal not in range(128)
> make: *** [docs] Error 1
>
>
> On Tue, Oct 30, 2012 at 1:26 PM, Danny Hermes <dhermes@google.com> wrote:
>
>> Patch uploaded. There are still some docs that end up different, but they
>> seem to have been patched by human hands.
>>
>> Also, all of a sudden, make docs is failing, complaining about a right
>> tick mark (\u2019) when it tries to right to a file with no codecs. I
>> checked and double-checked and this has not been introduced in my code.
>>
>> Have you guys experienced this?
>>
>>
>> On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com>wrote:
>>
>>> Yes. That is what I meant. Will update with a patch.
>>>
>>>
>>> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio
<jcgregorio@google.com>wrote:
>>>
>>>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com>
>>>> wrote:
>>>> > It does change things. For example it is removing all the
>>>> subresources from
>>>> > specific API docs.
>>>> >
>>>> > This is likely because the attr is not set on the object so
>>>> object.__dict__
>>>> > does not get updated and things like dir(service_object) won't list
>>>> the
>>>> > subresources.
>>>>
>>>> Yeah, definitely want to keep the dir(service_object) behavior
>>>> unchanged.
>>>>
>>>> > I can set this in a method that will also set it in _attr_cache or
>>>> just
>>>> > track the attributes that would have ended up in attr_cache via some
>>>> other
>>>> > mechanism.
>>>>
>>>> Or would it be easier to invert the meaning of attr_cache and have it
>>>> just be the name of attributes that will be stripped out of the
>>>> response from
>>>> __getstate__? That way you can also avoid over-riding __getattribute__.
>>>>
>>>>  -joe
>>>>
>>>> >
>>>> > What do you think?
>>>> >
>>>> >
>>>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com
>>>> >
>>>> > wrote:
>>>> >>
>>>> >> There's a Makefile in the project. Just run:
>>>> >>
>>>> >> $ make docs
>>>> >>
>>>> >> Then look at the diffs in the docs that produces. You probably want
>>>> to
>>>> >> patch this CL into
>>>> >> another copy of the repository since it will modify the doc files
>>>> >> which you probably don't want
>>>> >> as a part of this CL.
>>>> >>
>>>> >>   Thanks,
>>>> >>   -joe
>>>> >>
>>>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com>
>>>> wrote:
>>>> >> > Nope. How might one do this?
>>>> >> >
>>>> >> >
>>>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <
>>>> jcgregorio@google.com>
>>>> >> > wrote:
>>>> >> >>
>>>> >> >> Have you also verified that this doesn't change the output of
>>>> 'make
>>>> >> >> docs'
>>>> >> >> ?
>>>> >> >>
>>>> >> >>   -joe
>>>> >> >>
>>>> >> >> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
>>>> >> >> > I have already updated the new patch set.
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
>>>> >> >> > File apiclient/discovery.py (right):
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>>> >> >> > apiclient/discovery.py:384:
>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>>> >> >> >>
>>>> >> >> >> one blank line
>>>> >> >> >
>>>> >> >> >
>>>> >> >> > Done.
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>>> >> >> > apiclient/discovery.py:634: previous_response: The response
>>>> from the
>>>> >> >> > request for the previous page. (required)
>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>>> >> >> >>
>>>> >> >> >> 80 chars
>>>> >> >> >
>>>> >> >> >
>>>> >> >> > Done.
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None,
>>>> >> >> > baseUrl=None,
>>>> >> >> > model=None, developerKey=None,
>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>>> >> >> >>
>>>> >> >> >> I don't think any of these needs =None defaults.
>>>> >> >> >
>>>> >> >> >
>>>> >> >> > You are right. I'm not sure why I changed this, and also
>>>> accidentally
>>>> >> >> > changed the order of developerKey and requestBuilder.
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>>
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>>> >> >> > apiclient/discovery.py:739: def set_service_methods(self):
>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>>> >> >> >>
>>>> >> >> >> _set_service_methods
>>>> >> >> >
>>>> >> >> >
>>>> >> >> > Done.
>>>> >> >> >
>>>> >> >> > https://codereview.appspot.com/6610049/
>>>> >> >
>>>> >> >
>>>> >> >
>>>> >> >
>>>> >> > --
>>>> >> > Danny Hermes
>>>> >> > Developer Programs Engineer
>>>> >
>>>> >
>>>> >
>>>> >
>>>> > --
>>>> > Danny Hermes
>>>> > Developer Programs Engineer
>>>>
>>>
>>>
>>>
>>> --
>>> Danny Hermes
>>> Developer Programs Engineer
>>>
>>
>>
>>
>> --
>> Danny Hermes
>> Developer Programs Engineer
>>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer
>



-- 
Danny Hermes
Developer Programs Engineer

Sign in to reply to this message.

jcgregorio_google

The fix for the encoding issue is: diff -r c58ae21bb5e0 describe.py --- a/describe.py Mon Nov ...

11 years, 10 months ago (2012-11-05 13:05:21 UTC) #19

The fix for the encoding issue is:

diff -r c58ae21bb5e0 describe.py
--- a/describe.py       Mon Nov 05 08:01:29 2012 -0500
+++ b/describe.py       Mon Nov 05 08:03:30 2012 -0500
@@ -301,7 +301,7 @@
   html = document_collection(resource, path, root_discovery, discovery)

   f = open(os.path.join(BASE, path + 'html'), 'w')
-  f.write(html)
+  f.write(html.encode('utf-8'))
   f.close()

   for name in dir(resource):


When I generate all the docs I am still seeing issues. For example,
the arguments
are missing from list_next().

  -joe


On Wed, Oct 31, 2012 at 6:30 PM, Danny Hermes <dhermes@google.com> wrote:
> Can you try building the docs with a working version of make docs?
>
>
> On Tue, Oct 30, 2012 at 1:28 PM, Danny Hermes <dhermes@google.com> wrote:
>>
>> Traceback:
>>
>> python describe.py
>> Traceback (most recent call last):
>>   File "describe.py", line 346, in <module>
>>     document_api(api['name'], api['version'])
>>   File "describe.py", line 335, in document_api
>>     service, '%s_%s.' % (name, version), discovery, discovery)
>>   File "describe.py", line 314, in document_collection_recursive
>>     discovery['resources'].get(dname, {}))
>>   File "describe.py", line 304, in document_collection_recursive
>>     f.write(html)
>> UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in
>> position 5677: ordinal not in range(128)
>> make: *** [docs] Error 1
>>
>>
>> On Tue, Oct 30, 2012 at 1:26 PM, Danny Hermes <dhermes@google.com> wrote:
>>>
>>> Patch uploaded. There are still some docs that end up different, but they
>>> seem to have been patched by human hands.
>>>
>>> Also, all of a sudden, make docs is failing, complaining about a right
>>> tick mark (\u2019) when it tries to right to a file with no codecs. I
>>> checked and double-checked and this has not been introduced in my code.
>>>
>>> Have you guys experienced this?
>>>
>>>
>>> On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com>
>>> wrote:
>>>>
>>>> Yes. That is what I meant. Will update with a patch.
>>>>
>>>>
>>>> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>
>>>> wrote:
>>>>>
>>>>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com>
>>>>> wrote:
>>>>> > It does change things. For example it is removing all the
>>>>> > subresources from
>>>>> > specific API docs.
>>>>> >
>>>>> > This is likely because the attr is not set on the object so
>>>>> > object.__dict__
>>>>> > does not get updated and things like dir(service_object) won't list
>>>>> > the
>>>>> > subresources.
>>>>>
>>>>> Yeah, definitely want to keep the dir(service_object) behavior
>>>>> unchanged.
>>>>>
>>>>> > I can set this in a method that will also set it in _attr_cache or
>>>>> > just
>>>>> > track the attributes that would have ended up in attr_cache via some
>>>>> > other
>>>>> > mechanism.
>>>>>
>>>>> Or would it be easier to invert the meaning of attr_cache and have it
>>>>> just be the name of attributes that will be stripped out of the
>>>>> response from
>>>>> __getstate__? That way you can also avoid over-riding __getattribute__.
>>>>>
>>>>>  -joe
>>>>>
>>>>> >
>>>>> > What do you think?
>>>>> >
>>>>> >
>>>>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio
>>>>> > <jcgregorio@google.com>
>>>>> > wrote:
>>>>> >>
>>>>> >> There's a Makefile in the project. Just run:
>>>>> >>
>>>>> >> $ make docs
>>>>> >>
>>>>> >> Then look at the diffs in the docs that produces. You probably want
>>>>> >> to
>>>>> >> patch this CL into
>>>>> >> another copy of the repository since it will modify the doc files
>>>>> >> which you probably don't want
>>>>> >> as a part of this CL.
>>>>> >>
>>>>> >>   Thanks,
>>>>> >>   -joe
>>>>> >>
>>>>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com>
>>>>> >> wrote:
>>>>> >> > Nope. How might one do this?
>>>>> >> >
>>>>> >> >
>>>>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio
>>>>> >> > <jcgregorio@google.com>
>>>>> >> > wrote:
>>>>> >> >>
>>>>> >> >> Have you also verified that this doesn't change the output of
>>>>> >> >> 'make
>>>>> >> >> docs'
>>>>> >> >> ?
>>>>> >> >>
>>>>> >> >>   -joe
>>>>> >> >>
>>>>> >> >> On Tue, Oct 30, 2012 at 2:12 PM,  <dhermes@google.com> wrote:
>>>>> >> >> > I have already updated the new patch set.
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py
>>>>> >> >> > File apiclient/discovery.py (right):
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>>>> >> >> > apiclient/discovery.py:384:
>>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>>>> >> >> >>
>>>>> >> >> >> one blank line
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> > Done.
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>>>> >> >> > apiclient/discovery.py:634: previous_response: The response
>>>>> >> >> > from the
>>>>> >> >> > request for the previous page. (required)
>>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>>>> >> >> >>
>>>>> >> >> >> 80 chars
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> > Done.
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>>>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None,
>>>>> >> >> > baseUrl=None,
>>>>> >> >> > model=None, developerKey=None,
>>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>>>> >> >> >>
>>>>> >> >> >> I don't think any of these needs =None defaults.
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> > You are right. I'm not sure why I changed this, and also
>>>>> >> >> > accidentally
>>>>> >> >> > changed the order of developerKey and requestBuilder.
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> >
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco...
>>>>> >> >> > apiclient/discovery.py:739: def set_service_methods(self):
>>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote:
>>>>> >> >> >>
>>>>> >> >> >> _set_service_methods
>>>>> >> >> >
>>>>> >> >> >
>>>>> >> >> > Done.
>>>>> >> >> >
>>>>> >> >> > https://codereview.appspot.com/6610049/
>>>>> >> >
>>>>> >> >
>>>>> >> >
>>>>> >> >
>>>>> >> > --
>>>>> >> > Danny Hermes
>>>>> >> > Developer Programs Engineer
>>>>> >
>>>>> >
>>>>> >
>>>>> >
>>>>> > --
>>>>> > Danny Hermes
>>>>> > Developer Programs Engineer
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Danny Hermes
>>>> Developer Programs Engineer
>>>
>>>
>>>
>>>
>>> --
>>> Danny Hermes
>>> Developer Programs Engineer
>>
>>
>>
>>
>> --
>> Danny Hermes
>> Developer Programs Engineer
>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer

Sign in to reply to this message.

dhermes

Thanks. I figured out the issue. Your parser in describe.method_params was assuming 0 padding in ...

11 years, 10 months ago (2012-11-05 17:47:46 UTC) #20

jcgregorio_google

LGTM On 2012/11/05 17:47:46, dhermes wrote: > Thanks. I figured out the issue. Your parser ...

11 years, 10 months ago (2012-11-06 17:04:46 UTC) #21

jcgregorio_google

Committed in http://code.google.com/p/google-api-python-client/source/detail?r=5d8ad3d7d2b6cf52d1df1f4a49737e776708e446 That was an epic refactoring, thanks for sticking with it. -joe On ...

11 years, 9 months ago (2012-11-20 19:31:24 UTC) #22

dhermes

Woo hoo!! On Tue, Nov 20, 2012 at 11:31 AM, <jcgregorio@google.com> wrote: > Committed in ...

11 years, 9 months ago (2012-11-20 19:32:20 UTC) #23

Ali Afshar

11 years, 9 months ago (2012-11-20 19:34:06 UTC) #24

+1


On Tue, Nov 20, 2012 at 11:31 AM, Danny Hermes <dhermes@google.com> wrote:

> Woo hoo!!
>
>
> On Tue, Nov 20, 2012 at 11:31 AM, <jcgregorio@google.com> wrote:
>
>> Committed in
>> http://code.google.com/p/**google-api-python-client/**source/detail?r=**
>>
5d8ad3d7d2b6cf52d1df1f4a49737e**776708e446<http://code.google.com/p/google-api-python-client/source/detail?r=5d8ad3d7d2b6cf52d1df1f4a49737e776708e446>
>>
>> That was an epic refactoring, thanks for sticking with it.
>>
>>  -joe
>>
>>
>> On 2012/11/06 17:04:46, jcgregorio_google wrote:
>>
>>> LGTM
>>>
>>
>>  On 2012/11/05 17:47:46, dhermes wrote:
>>> > Thanks. I figured out the issue. Your parser in
>>>
>> describe.method_params was
>>
>>> > assuming 0 padding in docstrings, and I was indenting so it looked
>>>
>> like a
>>
>>> > docstring.
>>> >
>>> > For this commit, let's leave the parser alone, but then fix it for
>>>
>> the next go
>>
>>> > around.
>>> >
>>> > Since I aligned the triple quotes, all of the dyn html files have a
>>>
>> closing
>>
>>> pre
>>> > tag with 2 extra spaces because our nextMethod definitions occur at
>>>
>> different
>>
>>> > indent levels.
>>>
>>
>>
>>
https://codereview.appspot.**com/6610049/<https://codereview.appspot.com/6610...
>>
>
>
>
> --
> Danny Hermes
> Developer Programs Engineer
>



-- 
Ali Afshar | www.googplus.org/ali | Google Developer Relations

Sign in to reply to this message.

Issue 6610049: Moving Resource class to module level to make it serializable. (Closed)

Patch Set 1 #

Patch Set 2 : Removing commented out code that was left in by mistake. #

Patch Set 3 : Addressing review comments from jcgregorio. #

Patch Set 4 : Changing attr cache to just keep track of the dynamic attributes. #

Patch Set 5 : Adding docstring to _set_dynamic_attr method. #

Patch Set 6 : Changed indent on docstring in methodNext, was causing parser to fail in describe.method_params. #

Patch Set 7 : Fixing pickle tests in discovery after changing from _attr_cache to _dynamic_attr list. #

Patch Set 8 : Changing setup.py to target a newer version of httplib2. #

Patch Set 9 : Rebasing after several commits. #

Messages