Sample to list all user members from a group

[alainv, 2012-01-20 23:44:47]

http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... File samples/apps/list_group_members.py (right): http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:17: """Sample to list all the user members of a group. s/user // http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:37: """Get an OAuth 2.0 for the given credentials. "Get an OAuth 2.0 token using the provided client_id and client_secret." Also, add a link to the API console explaining how to get those 2 values. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:40: client_id: String client id of the developer. s/String/String,/ Same below. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:58: """Lists all user members including the users from member groups. "Lists all members including members of sub-groups." http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:67: group_members = group_client.RetrieveAllMembers(group_id) Add try/except block to catch API errors. Also, out of curiosity, does this handle paging? http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:71: # Assuming there is no cycle, i.e. groups are not members of each other As an improvement, you can use a set of visited group ID in order to avoid cycles and maybe use a max_depth variable as well. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:79: usage = 'Usage: %prog [options]' If the arguments are mandatory, there are not options. Please change the usage. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:92: and options.GROUP_ID): Fix indentation.

[Vic Fryzel, 2012-01-21 03:27:09]

http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... File samples/apps/list_group_members.py (right): http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:31: 2 lines between imports and first statement http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:56: Again, 2 lines between all top-level members. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:78: def main(): docstring

[shraddhag, 2012-01-21 10:58:03]

http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... File samples/apps/list_group_members.py (right): http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:17: """Sample to list all the user members of a group. On 2012/01/20 23:44:47, alainv wrote: > s/user // Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:31: On 2012/01/21 03:27:09, Vic Fryzel wrote: > 2 lines between imports and first statement Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:31: On 2012/01/21 03:27:09, Vic Fryzel wrote: > 2 lines between imports and first statement Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:37: """Get an OAuth 2.0 for the given credentials. On 2012/01/20 23:44:47, alainv wrote: > "Get an OAuth 2.0 token using the provided client_id and client_secret." > > Also, add a link to the API console explaining how to get those 2 values. Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:40: client_id: String client id of the developer. On 2012/01/20 23:44:47, alainv wrote: > s/String/String,/ > > Same below. Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:56: On 2012/01/21 03:27:09, Vic Fryzel wrote: > Again, 2 lines between all top-level members. Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:58: """Lists all user members including the users from member groups. On 2012/01/20 23:44:47, alainv wrote: > "Lists all members including members of sub-groups." Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:67: group_members = group_client.RetrieveAllMembers(group_id) On 2012/01/20 23:44:47, alainv wrote: > Add try/except block to catch API errors. > > Also, out of curiosity, does this handle paging? Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:71: # Assuming there is no cycle, i.e. groups are not members of each other On 2012/01/20 23:44:47, alainv wrote: > As an improvement, you can use a set of visited group ID in order to avoid > cycles and maybe use a max_depth variable as well. Tested and found out, groups cannot contain cycle, so removed the comment also. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:78: def main(): On 2012/01/21 03:27:09, Vic Fryzel wrote: > docstring Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:79: usage = 'Usage: %prog [options]' On 2012/01/20 23:44:47, alainv wrote: > If the arguments are mandatory, there are not options. Please change the usage. Done. http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:92: and options.GROUP_ID): On 2012/01/20 23:44:47, alainv wrote: > Fix indentation. It is 4 places indented due to continuation from previous line. This is allowed as per style guide. Yet, indenting the continued line to align with the parenthesis.

[shraddhag, 2012-01-21 11:39:22]

http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... File samples/apps/list_group_members.py (right): http://codereview.appspot.com/5536049/diff/2002/samples/apps/list_group_membe... samples/apps/list_group_members.py:67: group_members = group_client.RetrieveAllMembers(group_id) On 2012/01/21 10:58:04, shraddhag wrote: > On 2012/01/20 23:44:47, alainv wrote: > > Add try/except block to catch API errors. > > > > Also, out of curiosity, does this handle paging? > > Done. Yes, it handles paging.

[alainv, 2012-01-23 18:57:49]

Fix the remaining 2 small comments and LGTM! http://codereview.appspot.com/5536049/diff/6002/samples/apps/list_group_membe... File samples/apps/list_group_members.py (right): http://codereview.appspot.com/5536049/diff/6002/samples/apps/list_group_membe... samples/apps/list_group_members.py:35: """Get an OAuth 2.0 using the provided client_id and client_secret. Missing the "access token" after OAuth 2.0 (this might exceed 80 characters so re-phrase as needed). http://codereview.appspot.com/5536049/diff/6002/samples/apps/list_group_membe... samples/apps/list_group_members.py:45: token: String, authorized OAuth 2.0 token Missing period at the end of the sentence.

[shraddhag, 2012-01-24 06:22:19]

http://codereview.appspot.com/5536049/diff/6002/samples/apps/list_group_membe... File samples/apps/list_group_members.py (right): http://codereview.appspot.com/5536049/diff/6002/samples/apps/list_group_membe... samples/apps/list_group_members.py:35: """Get an OAuth 2.0 using the provided client_id and client_secret. On 2012/01/23 18:57:49, alainv wrote: > Missing the "access token" after OAuth 2.0 (this might exceed 80 characters so > re-phrase as needed). Done. http://codereview.appspot.com/5536049/diff/6002/samples/apps/list_group_membe... samples/apps/list_group_members.py:45: token: String, authorized OAuth 2.0 token On 2012/01/23 18:57:49, alainv wrote: > Missing period at the end of the sentence. Done.

[Vic Fryzel, 2012-01-26 03:54:25]

LGTM