|
|
|
Created:
14 years, 4 months ago by shraddhag Modified:
14 years, 4 months ago CC:
gdata-python-client-library-contributors_googlegroups.com Visibility:
Public. |
Patch Set 1 #
Total comments: 32
Patch Set 2 : Addressed the comments #
Total comments: 4
Patch Set 3 : addressed the comments #MessagesTotal messages: 8
Out of curiosity, what is the purpose of this sample? There is already a sample in the repository showing how to use 2-LO & batch requests on the Profiles API. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... File samples/apps/user_profile_contacts_2lo.py (right): http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:35: """Object to demonstrate retrieval of domain user's profile and contacts.""" s/Object/Class/ http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:50: requestor_id = 'admin@' + self.consumer_key This requires that the domain has an admin account named "admin". Shouldn't you use the user_id? http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:54: self.client = gdata.apps.client.AppsClient( Rename this variable to remove confusion (e.g "apps_client"). http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:70: print 'Email: %s (primary)' % (email.address) No need for parenthesis around email.address, apply throughout. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:93: requestor_id = 'admin@' + self.domain Same comment about admin account. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:95: entry_uri = self.contacts_client.GetFeedUri('profiles')+'/'+user_id Add spaces around the '+' operator. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:126: user = self.client.RetrieveUser(user_id) What is the point of using this API when the same information is returned by the Profiles API? (the question can be applied the other way around). http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:129: profile = self.GetProfile(user.login.user_name) Can the user_id and user.login.user_name be different? http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:134: print contact You can use PrintProfile to print the content of the contact entry instead of printing raw XML. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:140: usage = 'Usage: %prog [options]' Are the arguments optional? If not, put them in the Usage: Usage: %prog --CONSUMER_KEY=<> --CONSUMER_SECRET=<> --USER_ID=<> http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:151: options.USER_ID): Fix indentation. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:153: return Return an error code. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:155: options.CONSUMER_SECRET) Fix indentation. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:157: Add one blank line.
Sign in to reply to this message.
The purpose of this sample is to show 2LO for User Provisioning and how to retrieve profile and contacts info for a given user. I had only seen the Profile sample that uses password, not the 2LO one before. But I see the purpose of that sample is wider than just 2LO. Do you suggest to include anything in the sample to make it more useful? http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... File samples/apps/user_profile_contacts_2lo.py (right): http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:35: """Object to demonstrate retrieval of domain user's profile and contacts.""" On 2012/01/20 23:36:47, alainv wrote: > s/Object/Class/ object is required keyword in parameter. Class gives an error. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:50: requestor_id = 'admin@' + self.consumer_key On 2012/01/20 23:36:47, alainv wrote: > This requires that the domain has an admin account named "admin". > Shouldn't you use the user_id? Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:54: self.client = gdata.apps.client.AppsClient( On 2012/01/20 23:36:47, alainv wrote: > Rename this variable to remove confusion (e.g "apps_client"). Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:70: print 'Email: %s (primary)' % (email.address) On 2012/01/20 23:36:47, alainv wrote: > No need for parenthesis around email.address, apply throughout. Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:93: requestor_id = 'admin@' + self.domain On 2012/01/20 23:36:47, alainv wrote: > Same comment about admin account. Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:93: requestor_id = 'admin@' + self.domain On 2012/01/20 23:36:47, alainv wrote: > Same comment about admin account. Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:95: entry_uri = self.contacts_client.GetFeedUri('profiles')+'/'+user_id On 2012/01/20 23:36:47, alainv wrote: > Add spaces around the '+' operator. Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:126: user = self.client.RetrieveUser(user_id) On 2012/01/20 23:36:47, alainv wrote: > What is the point of using this API when the same information is returned by the > Profiles API? (the question can be applied the other way around). Retrieving name from the Profile API now and login data from the Provisioning API. I want to show 2LO for retrieving all user info from both APIs. Login data is not available from the Profiles API so included that. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:129: profile = self.GetProfile(user.login.user_name) On 2012/01/20 23:36:47, alainv wrote: > Can the user_id and user.login.user_name be different? Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:134: print contact On 2012/01/20 23:36:47, alainv wrote: > You can use PrintProfile to print the content of the contact entry instead of > printing raw XML. This is not printing raw XML. GetContacts() returns a list of contacts (title) http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:140: usage = 'Usage: %prog [options]' On 2012/01/20 23:36:47, alainv wrote: > Are the arguments optional? If not, put them in the Usage: > Usage: %prog --CONSUMER_KEY=<> --CONSUMER_SECRET=<> --USER_ID=<> Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:151: options.USER_ID): On 2012/01/20 23:36:47, alainv wrote: > Fix indentation. 4 places indentation is also allowed by the style guide. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:151: options.USER_ID): On 2012/01/20 23:36:47, alainv wrote: > Fix indentation. Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:153: return On 2012/01/20 23:36:47, alainv wrote: > Return an error code. Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:155: options.CONSUMER_SECRET) On 2012/01/20 23:36:47, alainv wrote: > Fix indentation. Done. http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:157: On 2012/01/20 23:36:47, alainv wrote: > Add one blank line. Done.
Sign in to reply to this message.
http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... File samples/apps/user_profile_contacts_2lo.py (right): http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:35: """Object to demonstrate retrieval of domain user's profile and contacts.""" On 2012/01/21 13:02:18, shraddhag wrote: > On 2012/01/20 23:36:47, alainv wrote: > > s/Object/Class/ > > object is required keyword in parameter. Class gives an error. Please further explain your previous comment. http://codereview.appspot.com/5554074/diff/3001/samples/apps/user_profile_con... File samples/apps/user_profile_contacts_2lo.py (right): http://codereview.appspot.com/5554074/diff/3001/samples/apps/user_profile_con... samples/apps/user_profile_contacts_2lo.py:51: requestor_id = admin_id + '@' + self.consumer_key requestor_id = '%s@%s' % (admin_id, self.consumer_key) http://codereview.appspot.com/5554074/diff/3001/samples/apps/user_profile_con... samples/apps/user_profile_contacts_2lo.py:55: self.apps_client = gdata.apps.client.AppsClient( Can this be made into a single line < 80 chars?
Sign in to reply to this message.
http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... File samples/apps/user_profile_contacts_2lo.py (right): http://codereview.appspot.com/5554074/diff/1/samples/apps/user_profile_contac... samples/apps/user_profile_contacts_2lo.py:35: """Object to demonstrate retrieval of domain user's profile and contacts.""" On 2012/01/22 06:50:49, Vic Fryzel wrote: > On 2012/01/21 13:02:18, shraddhag wrote: > > On 2012/01/20 23:36:47, alainv wrote: > > > s/Object/Class/ > > > > object is required keyword in parameter. Class gives an error. > > Please further explain your previous comment. Do I need to further explain the doc string for the class? http://codereview.appspot.com/5554074/diff/3001/samples/apps/user_profile_con... File samples/apps/user_profile_contacts_2lo.py (right): http://codereview.appspot.com/5554074/diff/3001/samples/apps/user_profile_con... samples/apps/user_profile_contacts_2lo.py:51: requestor_id = admin_id + '@' + self.consumer_key On 2012/01/22 06:50:49, Vic Fryzel wrote: > requestor_id = '%s@%s' % (admin_id, self.consumer_key) Done. http://codereview.appspot.com/5554074/diff/3001/samples/apps/user_profile_con... samples/apps/user_profile_contacts_2lo.py:55: self.apps_client = gdata.apps.client.AppsClient( On 2012/01/22 06:50:49, Vic Fryzel wrote: > Can this be made into a single line < 80 chars? Done.
Sign in to reply to this message.
|
