Code review request

[Takashi Matsuo]

Hi Jeff and list,

I'd like you to review my patch for retrying capability in some
methods related to retrieving all users on a domain.
Please see attached patch.

This patch will solve a problem that is originally reported in this thread:
http://groups.google.com/group/gdata-python-client-library-contributors/browse_thread/thread/a204d46342167862?hl=en

If its alright, I'd like to start implementing other 'RetrieveAll*'
methods as well.

Regards,

--
Takashi Matsuo
The father of kay framework

[wbc]

+1 to this from me. I have been using the retry code (thanks!) for
some days when trying to retrieve the membership of some very large
groups. It 'just works' for the most part! Although I need to look
into widening the scope of the retry events; currently it'll correctly
handle the HTTPS 'IncompleteRead' problems (which I'm now fairly
convinced are a client side Python/OpenSSL issue), but transient back-
end problems are not captured and retried. A 'quota exceeded' or
'unknown server error' still slip through and should really be
retried?

Many thanks, great work!
Will.

On Oct 1, 1:47 am, Takashi Matsuo <matsuo.taka...@gmail.com> wrote:
> Hi Jeff and list,
>
> I'd like you to review my patch for retrying capability in some
> methods related to retrieving all users on a domain.
> Please see attached patch.
>

> This patch will solve a problem that is originally reported in this thread:http://groups.google.com/group/gdata-python-client-library-contributo...

>
> If its alright, I'd like to start implementing other 'RetrieveAll*'
> methods as well.
>
> Regards,
>
> --
> Takashi Matsuo
> The father of kay framework
>

>  gdata-retrying-final.diff
> 7KViewDownload

[Takashi Matsuo]

Hi Will,

Thanks for your reply.
I've finished implementing GetGeneratorFor*** methods, so attached a patch.

This is a patch:
gdata-generator-methods-with-retries.diff

This is a data file for unittests:
gdata-test-for-generators-pickle.tar.gz

Jeff, could you consider incorporating this patch?

Regards,

--
Takashi Matsuo
Kay's daddy

[wbc]

I have just patched these changes into my libs and will test today!

Many Thanks,
Will.

>  gdata-generator-methods-with-retries.diff
> 22KViewDownload
>
>  gdata-test-for-generators-pickle.tar.gz
> 19KViewDownload

[wbc]

So... The patch as it stands doesn't help me personally; I'm working
with Groups and they subclass off the Properties stuff (which the
patch doesn't address)... Anyway, I hacked that class to use the 'Get
with Retries' in place of a straight 'Get' and it works just fine. I
guess that I should look into adding Generators there too?

Will.

[Takashi Matsuo]

Hi Will,

Thanks for testing. I'm happy with hearing that GetWithRetries method
works as designed.
I'm sorry that I forgot about GroupsService.

I'd rather say there is no serious need for Generator based method on
retrieving all groups because in most cases, the number of groups is
not so huge as the number of users. Having said that, having generator
based method is certainly worthy.

I'll work on that if there's a time, but I'm afraid there isn't.

Regards,

--
Takashi Matsuo
Kay's daddy

[Jeff S]

Hi Takashi Matsuo,

I checked in the changes to the service.py and the unit test. Do I
need to check in the pickle files as well? I wasn't sure whether these
were optional, to speed up the tests, or required. Thanks for making
these changes!

Happy coding,

Jeff

On Oct 15, 6:23 am, Takashi Matsuo <matsuo.taka...@gmail.com> wrote:
> Hi Will,
>
> Thanks for testing. I'm happy with hearing that GetWithRetries method
> works as designed.
> I'm sorry that I forgot about GroupsService.
>
> I'd rather say there is no serious need for Generator based method on
> retrieving all groups because in most cases, the number of groups is
> not so huge as the number of users. Having said that, having generator
> based method is certainly worthy.
>
> I'll work on that if there's a time, but I'm afraid there isn't.
>
> Regards,
>
> --
> Takashi Matsuo
> Kay's daddy
>