Issue 5376102: Added a sample that is a quick start guide for multidomain user provisioning

Claudio Cherubino

http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py File samples/apps/multidomain_quick_start_guide.py (right): http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode24 samples/apps/multidomain_quick_start_guide.py:24: import gdata.apps.multidomain.client did you run lint on this file? ...

12 years, 5 months ago (2011-11-15 17:46:48 UTC) #2

Vic Fryzel

There are too many reviewers for this change, can you reduce the set to 1 ...

12 years, 5 months ago (2011-11-15 18:06:05 UTC) #3

There are too many reviewers for this change, can you reduce the set to 1
or 2 people?

-Vic



On Tue, Nov 15, 2011 at 12:46 PM, <ccherubino@google.com> wrote:

>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py>
> File samples/apps/multidomain_**quick_start_guide.py (right):
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode24<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode24>
> samples/apps/multidomain_**quick_start_guide.py:24: import
> gdata.apps.multidomain.client
> did you run lint on this file? I think the imports are out of order
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode26<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode26>
> samples/apps/multidomain_**quick_start_guide.py:26: SCOPE =
>
'https://apps-apis.google.com/**a/feeds/user/<https://apps-apis.google.com/a/feeds/user/>
> '
> isn't this scope defined elsewhere?
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode28<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode28>
> samples/apps/multidomain_**quick_start_guide.py:28: BOOLEAN_CHOICES = {
> an associative array for boolean values seems like a bad idea to me.
> What's wrong with booleans?
> If you really want to use this, hide the fact that you are wrapping a
> boolean and use values like "ENABLE", "DISABLE", "UNDEFINED", but this
> code makes me think that you are just using booleans improperly.
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode34<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode34>
> samples/apps/multidomain_**quick_start_guide.py:34: class
> MultiDomainQuickStartGuide(**object):
> I'd remove the word "Guide" here and everywhere else it is used. This is
> not exactly a guide
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode40<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode40>
> samples/apps/multidomain_**quick_start_guide.py:40: Takes an client_id,
> client_secret and domain to create an object for
> s/an client_id/a client_id
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode45<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode45>
> samples/apps/multidomain_**quick_start_guide.py:45: password: [string] The
> clientSecret of the developer.
> password?
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode60<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode60>
> samples/apps/multidomain_**quick_start_guide.py:60:
> self.multidomain_client =
> gdata.apps.multidomain.client.**MultiDomainProvisioningClient(
> long line?
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode69<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode69>
> samples/apps/multidomain_**quick_start_guide.py:69: print 'Id Admin: %s' %
> (entry.is_admin)
> s/Id Admin/Is Admin
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode79<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode79>
> samples/apps/multidomain_**quick_start_guide.py:79: entry: []
> describe the arguments here and in the other functions
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode87<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode87>
> samples/apps/multidomain_**quick_start_guide.py:87: """Modulates creation
> of a new user"""
> What about "Creates a new user account"?
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode114<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode114>
> samples/apps/multidomain_**quick_start_guide.py:114: print 'Password
> cannot be blank an atleast 8 characters long'
> s/Password cannot be blank an atleast 8 characters long/Password must be
> at least 8 characters long
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode142<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode142>
> samples/apps/multidomain_**quick_start_guide.py:142: """Modulates updation
> of a user"""
> I don't like "modulates", I don't really know what it means and it is
> not used in any code I have seen
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode144<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode144>
> samples/apps/multidomain_**quick_start_guide.py:144: email = ''
> the block that starts from here till line 197 is about the same thing
> that you have in the previous function.
> Can you extract a method to get all properties you need from the user
> and call it from both functions?
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode147<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode147>
> samples/apps/multidomain_**quick_start_guide.py:147: user_feed =
> self.multidomain_client.**RetrieveUser(email)
> this is a not a user_feed, but a user_entry
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode213<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode213>
> samples/apps/multidomain_**quick_start_guide.py:213: """Modulates retrival
> of a user"""
> s/retrival/retrieval
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode280<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode280>
> samples/apps/multidomain_**quick_start_guide.py:280: print 'HERE'
> really? :)
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode309<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode309>
> samples/apps/multidomain_**quick_start_guide.py:309: print ('Choose an
> option:\n'
> perhaps there's a way to use the same data structure to hold the
> methods' names and the text description for the users.
> Otherwise you may end up with two lists not in-sync
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode322<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode322>
> samples/apps/multidomain_**quick_start_guide.py:322: choice =
> int(raw_input())
> two spaces before "int" here?
>
> http://codereview.appspot.com/**5376102/diff/1/samples/apps/**
>
multidomain_quick_start_guide.**py#newcode326<http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode326>
> samples/apps/multidomain_**quick_start_guide.py:326: self._BadChoice()
> this method doesn't add anything, move the print statement here
>
>
http://codereview.appspot.com/**5376102/<http://codereview.appspot.com/5376102/>
>
> --
> You received this message because you are subscribed to the Google Groups
> "GData Python Client Library Contributors" group.
> To post to this group, send email to gdata-python-client-library-**
>
contributors@googlegroups.com<gdata-python-client-library-contributors@google...
> .
> To unsubscribe from this group, send email to gdata-python-client-library-
>
**contributors+unsubscribe@**googlegroups.com<gdata-python-client-library-con...
> .
> For more options, visit this group at http://groups.google.com/**
>
group/gdata-python-client-**library-contributors?hl=en<http://groups.google.com/group/gdata-python-client-library-contributors?hl=en>
> .
>
>

Sign in to reply to this message.

gunjansharma

http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py File samples/apps/multidomain_quick_start_guide.py (right): http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode24 samples/apps/multidomain_quick_start_guide.py:24: import gdata.apps.multidomain.client On 2011/11/15 17:46:48, Claudio Cherubino wrote: > ...

12 years, 5 months ago (2011-11-16 12:06:32 UTC) #5

Claudio Cherubino

http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py File samples/apps/multidomain_quick_start_guide.py (right): http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_start_guide.py#newcode24 samples/apps/multidomain_quick_start_guide.py:24: import gdata.apps.multidomain.client On 2011/11/16 12:06:32, gunjansharma wrote: > On ...

12 years, 5 months ago (2011-11-16 17:42:54 UTC) #7

http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_s...
File samples/apps/multidomain_quick_start_guide.py (right):

http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_s...
samples/apps/multidomain_quick_start_guide.py:24: import
gdata.apps.multidomain.client
On 2011/11/16 12:06:32, gunjansharma wrote:
> On 2011/11/15 17:46:48, Claudio Cherubino wrote:
> > did you run lint on this file? I think the imports are out of order
> gives correct

this is what glint returns me:


*********** File
/home/ccherubino/workspace/python/gdata/samples/apps/multidomain_quick_start_example.py
C6304:  1: Invalid header. Must include a copyright matching Copyright \d{4}
Google Inc\.(\s*#)?\s*All Rights Reserved\.
C0111:  1: Missing docstring
C6111:  1: Missing module docstring
W0611: 22: Unused import atom
C6007: 29: Space is not allowed before this ':'
C6005: 29: Continued line after an open paren/brace should indent by 4
BOOLEAN_CHOICES = {
    |
  1 : True,
C6007: 30: Space is not allowed before this ':'
C6005: 30: Continued line after an open paren/brace should indent by 4
BOOLEAN_CHOICES = {
    |
  2 : False,
C6007: 31: Space is not allowed before this ':'
C6005: 31: Continued line after an open paren/brace should indent by 4
BOOLEAN_CHOICES = {
    |
  3 : None
C6302: 33: Need 2 blank lines before top level class
C6108: 35: One-line docstring summary should end with ".", "?", or "!"
C6308: 36: Trailing whitespace
C6302: 49: Need 2 blank lines before top level class
C6310: 76: Line too long (90/80)
C6005: 77: Continued line after an open paren/brace should indent by 4
    self.multidomain_client =
gdata.apps.multidomain.client.MultiDomainProvisioningClient(
        |
            domain=self.domain, auth_token=token)
C6308: 78: Trailing whitespace
C6108: 80: One-line docstring summary should end with ".", "?", or "!"
C6113: 80: Args section is missing or does not match arg list.
Expected: entry
   Found: 
C6308: 81: Trailing whitespace
C6308: 85: Trailing whitespace
C6308: 94: Trailing whitespace
C6108: 96: One-line docstring summary should end with ".", "?", or "!"
C6308: 97: Trailing whitespace
C6308:101: Trailing whitespace
C6308:105: Trailing whitespace
C6308:114: Trailing whitespace
C6108:116: One-line docstring summary should end with ".", "?", or "!"
C6308:117: Trailing whitespace
C6004:121: Continued line does not align with previous
      user_data.email = raw_input('Enter a valid email address'
                                  |
                            '(username@domain.com): ')
C6006:137: Exactly one space is required before this '<'
C6006:137: Exactly one space is required after this '<'
C6308:141: Trailing whitespace
C6308:144: Trailing whitespace
C6308:151: Trailing whitespace
C6308:157: Trailing whitespace
C6108:159: One-line docstring summary should end with ".", "?", or "!"
C6308:160: Trailing whitespace
W0612:162:MultiDomainQuickStartExample._CreateUser: Unused variable 'response'
C6308:168: Trailing whitespace
C6108:170: One-line docstring summary should end with ".", "?", or "!"
C6308:171: Trailing whitespace
W6404:180:MultiDomainQuickStartExample._UpdateUser: Use 'is' instead of '=='
when comparing to None
W6404:182:MultiDomainQuickStartExample._UpdateUser: Use 'is' instead of '=='
when comparing to None
W6404:184:MultiDomainQuickStartExample._UpdateUser: Use 'is' instead of '=='
when comparing to None
W6404:186:MultiDomainQuickStartExample._UpdateUser: Use 'is' instead of '=='
when comparing to None
W6404:188:MultiDomainQuickStartExample._UpdateUser: Use 'is' instead of '=='
when comparing to None
W6404:190:MultiDomainQuickStartExample._UpdateUser: Use 'is' instead of '=='
when comparing to None
C6308:192: Trailing whitespace
C6308:194: Trailing whitespace
C6108:196: One-line docstring summary should end with ".", "?", or "!"
C6308:197: Trailing whitespace
C6308:204: Trailing whitespace
C6308:206: Trailing whitespace
C6108:208: One-line docstring summary should end with ".", "?", or "!"
C6308:209: Trailing whitespace
C6308:213: Trailing whitespace
C6308:216: Trailing whitespace
C6108:218: One-line docstring summary should end with ".", "?", or "!"
C6308:219: Trailing whitespace
C6308:223: Trailing whitespace
C6108:225: One-line docstring summary should end with ".", "?", or "!"
C6308:226: Trailing whitespace
C6308:230: Trailing whitespace
C6308:232: Trailing whitespace
C6108:234: One-line docstring summary should end with ".", "?", or "!"
C6308:235: Trailing whitespace
C6308:244: Trailing whitespace
C6108:246: One-line docstring summary should end with ".", "?", or "!"
C6308:247: Trailing whitespace
C6308:254: Trailing whitespace
C6108:256: One-line docstring summary should end with ".", "?", or "!"
C6308:257: Trailing whitespace
C6308:261: Trailing whitespace
C6108:263: One-line docstring summary should end with ".", "?", or "!"
C6308:264: Trailing whitespace
C6308:271: Trailing whitespace
C6108:273: One-line docstring summary should end with ".", "?", or "!"
C6308:274: Trailing whitespace
C6308:280: Trailing whitespace
C6108:282: One-line docstring summary should end with ".", "?", or "!"
C6308:283: Trailing whitespace
C6007:286: Space is not allowed before this ':'
C6005:286: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._CreateUser,
C6007:287: Space is not allowed before this ':'
C6004:287: Continued line does not align with previous
      {'function' : self._CreateUser,
       |
          'description' : 'Create a user'},
C6007:288: Space is not allowed before this ':'
C6005:288: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._UpdateUser,
C6007:289: Space is not allowed before this ':'
C6004:289: Continued line does not align with previous
      {'function' : self._UpdateUser,
       |
          'description' : 'Updating a user'},
C6007:290: Space is not allowed before this ':'
C6005:290: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._RenameUser,
C6007:291: Space is not allowed before this ':'
C6004:291: Continued line does not align with previous
      {'function' : self._RenameUser,
       |
          'description' : 'Renaming a user'},
C6007:292: Space is not allowed before this ':'
C6005:292: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._RetrieveSingleUser,
C6007:293: Space is not allowed before this ':'
C6004:293: Continued line does not align with previous
      {'function' : self._RetrieveSingleUser,
       |
          'description' : 'Retrieve a single user'},
C6007:294: Space is not allowed before this ':'
C6005:294: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._RetrieveAllUsers,
C6007:295: Space is not allowed before this ':'
C6004:295: Continued line does not align with previous
      {'function' : self._RetrieveAllUsers,
       |
          'description' : 'Retrieve all users in all domains'},
C6007:296: Space is not allowed before this ':'
C6005:296: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._DeleteUser,
C6007:297: Space is not allowed before this ':'
C6004:297: Continued line does not align with previous
      {'function' : self._DeleteUser,
       |
          'description' : 'Deleting a User from a domain'},
C6007:298: Space is not allowed before this ':'
C6005:298: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._CreateUserAlias,
C6007:299: Space is not allowed before this ':'
C6004:299: Continued line does not align with previous
      {'function' : self._CreateUserAlias,
       |
          'description' : 'Create a User Alias in a domain'},
C6007:300: Space is not allowed before this ':'
C6005:300: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._RetrieveAlias,
C6007:301: Space is not allowed before this ':'
C6004:301: Continued line does not align with previous
      {'function' : self._RetrieveAlias,
       |
          'description' : 'Retrieve a User Alias in a domain'},
C6007:302: Space is not allowed before this ':'
C6005:302: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._RetrieveAllAliases,
C6007:303: Space is not allowed before this ':'
C6004:303: Continued line does not align with previous
      {'function' : self._RetrieveAllAliases,
       |
          'description' : 'Retrieve all User Aliases in a Domain'},
C6007:304: Space is not allowed before this ':'
C6005:304: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._RetrieveAllUserAliases,
C6007:305: Space is not allowed before this ':'
C6004:305: Continued line does not align with previous
      {'function' : self._RetrieveAllUserAliases,
       |
          'description' : 'Retrieve all User Aliases for a User'},
C6007:306: Space is not allowed before this ':'
C6005:306: Continued line after an open paren/brace should indent by 4
    functions_list = [
        |
      {'function' : self._DeleteAlias,
C6007:307: Space is not allowed before this ':'
C6004:307: Continued line does not align with previous
      {'function' : self._DeleteAlias,
       |
          'description' : 'Delete a user alias from a domain'}
C6308:309: Trailing whitespace
W0301:316: Unnecessary semicolon
C6006:317: Exactly one space is required before this '<'
C6006:317: Exactly one space is required after this '<'
C6006:317: Exactly one space is required before this '>'
C6006:317: Exactly one space is required after this '>'
C6108:324: One-line docstring summary should end with ".", "?", or "!"
C6308:325: Trailing whitespace
W0612:328:main: Unused variable 'args'
W0612:331:main: Unused variable 'msg'

http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_s...
samples/apps/multidomain_quick_start_guide.py:26: SCOPE =
'https://apps-apis.google.com/a/feeds/user/'
On 2011/11/16 12:06:32, gunjansharma wrote:
> On 2011/11/15 17:46:48, Claudio Cherubino wrote:
> > isn't this scope defined elsewhere?
> Elsewhere as in?

http://code.google.com/p/gdata-python-client/source/browse/src/gdata/gauth.py
but you can leave it here for simplicity

http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_s...
samples/apps/multidomain_quick_start_guide.py:28: BOOLEAN_CHOICES = {
On 2011/11/15 17:46:48, Claudio Cherubino wrote:
> an associative array for boolean values seems like a bad idea to me. What's
> wrong with booleans? 
> If you really want to use this, hide the fact that you are wrapping a boolean
> and use values like "ENABLE", "DISABLE", "UNDEFINED", but this code makes me
> think that you are just using booleans improperly.

What do you want to do with this code?

http://codereview.appspot.com/5376102/diff/1/samples/apps/multidomain_quick_s...
samples/apps/multidomain_quick_start_guide.py:60: self.multidomain_client =
gdata.apps.multidomain.client.MultiDomainProvisioningClient(
On 2011/11/16 12:06:32, gunjansharma wrote:
> On 2011/11/15 17:46:48, Claudio Cherubino wrote:
> > long line?
> Don't think that it can be reduced.

What about moving gdata.apps.multidomain.client.MultiDomainProvisioningClient to
a new line with 4 spaces at the beginning?

http://codereview.appspot.com/5376102/diff/9001/samples/apps/multidomain_quic...
File samples/apps/multidomain_quick_start_example.py (right):

http://codereview.appspot.com/5376102/diff/9001/samples/apps/multidomain_quic...
samples/apps/multidomain_quick_start_example.py:62: domain: [string] The domain
on which the funtions are to be performed.
s/funtions/functions

http://codereview.appspot.com/5376102/diff/9001/samples/apps/multidomain_quic...
samples/apps/multidomain_quick_start_example.py:83: entry: [UserEntry] User
entry correspoding to a user
s/correspoding/corresponding

Sign in to reply to this message.

gunjansharma

http://codereview.appspot.com/5376102/diff/9001/samples/apps/multidomain_quick_start_example.py File samples/apps/multidomain_quick_start_example.py (right): http://codereview.appspot.com/5376102/diff/9001/samples/apps/multidomain_quick_start_example.py#newcode62 samples/apps/multidomain_quick_start_example.py:62: domain: [string] The domain on which the funtions are ...

12 years, 5 months ago (2011-11-16 17:59:29 UTC) #9

LGTM

Sign in to reply to this message.

Issue 5376102: Added a sample that is a quick start guide for multidomain user provisioning

Patch Set 1 #

Patch Set 2 : Resolved the comments and changed the file name #

Patch Set 3 : Long lines shortened #

Patch Set 4 : Resolved next set of comments #

Messages