Client and data model for Provisioning API

[Claudio Cherubino, 2011-10-31 16:41:43]

Please add unit tests for these new classes, you can use those for the MDM API
as reference:

http://code.google.com/p/gdata-python-client/source/browse/tests/gdata_tests/...

Also, remember to run lint on the files.

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py
File src/gdata/apps/client.py (right):

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode18
src/gdata/apps/client.py:18: __author__ = 'Shraddha Gupta shraddhag@google.com>'
missing opening <

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode45
src/gdata/apps/client.py:45: OAuthToken which authorizes this client to edit the
email settings.
email settings???

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode57
src/gdata/apps/client.py:57: return '/a/feeds/%s' % self.domain
move this method before the two others that use it

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode59
src/gdata/apps/client.py:59: def AddAllElementsFromAllPages(self, link_finder,
func):
I don't like the name, I'd probably use RetrieveAllPages.

Add the comments to explain what it does, the parameters and what it returns, I
don't know what type of object is a link_finder

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode71
src/gdata/apps/client.py:71: suspended='false', admin=None, quota_limit=None,
boolean values should be passed as booleans and then the library should take
care of conversions.

suspended should be a proper boolean

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode73
src/gdata/apps/client.py:73: agreedToTerms=None, change_password=None):
you are mixing camelCase style with words_separated_by_underlines

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode131
src/gdata/apps/client.py:131: user_entry: UserEntry updated user entry
should UserEntry be gdata.apps.data.UserEntry ?

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode138
src/gdata/apps/client.py:138: return self.update(entry=user_entry, uri=uri)
I'm confused by the method naming, you have "update" and "Post", one represents
the action, the other the HTTP method.
Not to mention the different capitalization

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/data.py
File src/gdata/apps/data.py (right):

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/data.py#newcode28
src/gdata/apps/data.py:28: ATOM_NAMESPACE = '{http://www.w3.org/2005/Atom}%s'
I'm pretty sure these two namespaces are already defined somewhere else in the
library

[shraddhag, 2011-11-03 17:04:41]

self.assertEquals(new_entry.login.password, 'password')

This command gives error.
new_entry.login.password is None (seems password does not get assigned or
cannot be accessed here)
Can you tell me what is the problem?

-
Shraddha




On Thu, Nov 3, 2011 at 10:22 PM, <shraddhag@google.com> wrote:

>
http://codereview.appspot.com/**5303054/<http://codereview.appspot.com/5303054/>
>

[Claudio Cherubino, 2011-11-03 17:40:00]

When you create an user, the entry that is returned to you doesn't include
the password for security reasons.
Actually, passwords are never returned by Google Apps.

Claudio

On Thu, Nov 3, 2011 at 10:04 AM, Shraddha Gupta <shraddhag@google.com>wrote:

> self.assertEquals(new_entry.login.password, 'password')
>
> This command gives error.
> new_entry.login.password is None (seems password does not get assigned or
> cannot be accessed here)
> Can you tell me what is the problem?
>
> -
> Shraddha
>
>
>
>
> On Thu, Nov 3, 2011 at 10:22 PM, <shraddhag@google.com> wrote:
>
>>
http://codereview.appspot.com/**5303054/<http://codereview.appspot.com/5303054/>
>>
>
>

[shraddhag, 2011-11-08 04:57:09]

Thanks. Means I can remove the assert for password. 
Please review the patch when you get time.

On 2011/11/03 17:40:00, Claudio Cherubino wrote:
> When you create an user, the entry that is returned to you doesn't include
> the password for security reasons.
> Actually, passwords are never returned by Google Apps.
> 
> Claudio
> 
> On Thu, Nov 3, 2011 at 10:04 AM, Shraddha Gupta <shraddhag@google.com>wrote:
> 
> > self.assertEquals(new_entry.login.password, 'password')
> >
> > This command gives error.
> > new_entry.login.password is None (seems password does not get assigned or
> > cannot be accessed here)
> > Can you tell me what is the problem?
> >
> > -
> > Shraddha
> >
> >
> >
> >
> > On Thu, Nov 3, 2011 at 10:22 PM, <mailto:shraddhag@google.com> wrote:
> >
> >>
>
http://codereview.appspot.com/**5303054/%3Chttp://codereview.appspot.com/5303...>
> >>
> >
> >

[shraddhag, 2011-11-08 04:57:47]

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py
File src/gdata/apps/client.py (right):

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode18
src/gdata/apps/client.py:18: __author__ = 'Shraddha Gupta shraddhag@google.com>'
On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> missing opening <

Done.

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode45
src/gdata/apps/client.py:45: OAuthToken which authorizes this client to edit the
email settings.
On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> email settings???

Done.

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode57
src/gdata/apps/client.py:57: return '/a/feeds/%s' % self.domain
On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> move this method before the two others that use it

Done.

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode59
src/gdata/apps/client.py:59: def AddAllElementsFromAllPages(self, link_finder,
func):
On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> I don't like the name, I'd probably use RetrieveAllPages.
> 
> Add the comments to explain what it does, the parameters and what it returns,
I
> don't know what type of object is a link_finder

Done.

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode71
src/gdata/apps/client.py:71: suspended='false', admin=None, quota_limit=None,
On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> boolean values should be passed as booleans and then the library should take
> care of conversions.
> 
> suspended should be a proper boolean

Done.

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode73
src/gdata/apps/client.py:73: agreedToTerms=None, change_password=None):
On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> you are mixing camelCase style with words_separated_by_underlines

Done.

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode131
src/gdata/apps/client.py:131: user_entry: UserEntry updated user entry
On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> should UserEntry be gdata.apps.data.UserEntry ?

Done.

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/client.py#newcode138
src/gdata/apps/client.py:138: return self.update(entry=user_entry, uri=uri)
Update and Post are the methods in GDClient class. Post here does not refer to
HTTP method but the method. To make changes to the name I will have to change
GDClient class.

On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> I'm confused by the method naming, you have "update" and "Post", one
represents
> the action, the other the HTTP method.
> Not to mention the different capitalization

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/data.py
File src/gdata/apps/data.py (right):

http://codereview.appspot.com/5303054/diff/1/src/gdata/apps/data.py#newcode28
src/gdata/apps/data.py:28: ATOM_NAMESPACE = '{http://www.w3.org/2005/Atom}%s'
On 2011/10/31 16:41:43, Claudio Cherubino wrote:
> I'm pretty sure these two namespaces are already defined somewhere else in the
> library

Done.

[Claudio Cherubino, 2011-11-08 05:02:44]

Address that single comment and run lint on all files, besides that LGTM

http://codereview.appspot.com/5303054/diff/5001/tests/gdata_tests/apps/live_c...
File tests/gdata_tests/apps/live_client_test.py (right):

http://codereview.appspot.com/5303054/diff/5001/tests/gdata_tests/apps/live_c...
tests/gdata_tests/apps/live_client_test.py:98: #  
self.assertEquals(new_entry.login.password, 'password')
remove this line