Issue 4641071: Adding support for the MDM Provisioning API
|
Created:
12 years, 10 months ago by Claudio Cherubino Modified:
12 years, 9 months ago CC:
gdata-python-client-library-contributors_googlegroups.com Visibility:
Public. |
Patch Set 1 #
Total comments: 12
Patch Set 2 : Addressed first round of comments #Patch Set 3 : Second round of comments #
MessagesTotal messages: 12
Sign in to reply to this message.
any comments on this patch?
Sign in to reply to this message.
On 2011/06/30 16:12:18, Claudio Cherubino wrote: > any comments on this patch? I just have a look to your patch and it seems good for me. Thanks for this patch, it will be very useful for large companies using Google Apps. Alexandre Vivien
Sign in to reply to this message.
Joe, Do I have your green light to commit this code? Thanks Claudio On Thu, Jun 30, 2011 at 2:09 PM, <alx.vivien@gmail.com> wrote: > On 2011/06/30 16:12:18, Claudio Cherubino wrote: > >> any comments on this patch? >> > > I just have a look to your patch and it seems good for me. > Thanks for this patch, it will be very useful for large companies using > Google Apps. > > Alexandre Vivien > > > http://codereview.appspot.com/**4641071/<http://codereview.appspot.com/4641071/> >
Sign in to reply to this message.
http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... File tests/gdata_tests/apps/multidomain/data_test.py (right): http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/data_test.py:23: import unittest sort imports http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/data_test.py:58: 'https://apps-apis.google.com/a/feeds/user/2.0/example.com/admin%40example.com') 80 chars here and other places. http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... File tests/gdata_tests/apps/multidomain/live_client_test.py (right): http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/live_client_test.py:46: if conf.options.get_value('ssl') == 'true': Does this API allow non-SSL access? If not then this should go in the base class. http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/live_client_test.py:103: new_entry = self.client.CreateUser( Can this fail?
Sign in to reply to this message.
http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... File tests/gdata_tests/apps/multidomain/data_test.py (right): http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/data_test.py:23: import unittest On 2011/07/12 01:21:29, jcgregorio wrote: > sort imports this is the order suggested by GPyLint http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/data_test.py:58: 'https://apps-apis.google.com/a/feeds/user/2.0/example.com/admin%40example.com') On 2011/07/12 01:21:29, jcgregorio wrote: > 80 chars > > here and other places. Done. http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... File tests/gdata_tests/apps/multidomain/live_client_test.py (right): http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/live_client_test.py:46: if conf.options.get_value('ssl') == 'true': On 2011/07/12 01:21:29, jcgregorio wrote: > Does this API allow non-SSL access? If not then this should go in the base > class. Non-SSL access is still allowed even if we only document the HTTPS endpoints http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/live_client_test.py:103: new_entry = self.client.CreateUser( On 2011/07/12 01:21:29, jcgregorio wrote: > Can this fail? Yes, it can fail, however we can't just delete the existing user and recreate it as deleted users can't be recreated for 5 days. To mitigate this issue I'm using a random username and we could also catch the exception, generate a new random number and try again (once? twice? n times? forever?) but we'll never get a bulletproof test.
Sign in to reply to this message.
Sign in to reply to this message.
Sign in to reply to this message.
http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... File tests/gdata_tests/apps/multidomain/live_client_test.py (right): http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/live_client_test.py:46: if conf.options.get_value('ssl') == 'true': Then let's switch the default to SSL and the user can change it back to non-SSL if they so choose. On 2011/07/12 20:42:27, Claudio Cherubino wrote: > On 2011/07/12 01:21:29, jcgregorio wrote: > > Does this API allow non-SSL access? If not then this should go in the base > > class. > > Non-SSL access is still allowed even if we only document the HTTPS endpoints http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/live_client_test.py:103: new_entry = self.client.CreateUser( Then let's up the randrange to 10,000 or higher. On 2011/07/12 20:42:27, Claudio Cherubino wrote: > On 2011/07/12 01:21:29, jcgregorio wrote: > > Can this fail? > > Yes, it can fail, however we can't just delete the existing user and recreate it > as deleted users can't be recreated for 5 days. > > To mitigate this issue I'm using a random username and we could also catch the > exception, generate a new random number and try again (once? twice? n times? > forever?) but we'll never get a bulletproof test.
Sign in to reply to this message.
http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... File tests/gdata_tests/apps/multidomain/live_client_test.py (right): http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/live_client_test.py:46: if conf.options.get_value('ssl') == 'true': The default for MultiDomainProvisioningClient is already SSL. On 2011/07/12 21:19:53, jcgregorio wrote: > Then let's switch the default to SSL and the user can change it back to non-SSL > if they so choose. > > On 2011/07/12 20:42:27, Claudio Cherubino wrote: > > On 2011/07/12 01:21:29, jcgregorio wrote: > > > Does this API allow non-SSL access? If not then this should go in the base > > > class. > > > > Non-SSL access is still allowed even if we only document the HTTPS endpoints > http://codereview.appspot.com/4641071/diff/1/tests/gdata_tests/apps/multidoma... tests/gdata_tests/apps/multidomain/live_client_test.py:103: new_entry = self.client.CreateUser( Set to 100,000 On 2011/07/12 21:19:53, jcgregorio wrote: > Then let's up the randrange to 10,000 or higher. > > On 2011/07/12 20:42:27, Claudio Cherubino wrote: > > On 2011/07/12 01:21:29, jcgregorio wrote: > > > Can this fail? > > > > Yes, it can fail, however we can't just delete the existing user and recreate > it > > as deleted users can't be recreated for 5 days. > > > > To mitigate this issue I'm using a random username and we could also catch the > > exception, generate a new random number and try again (once? twice? n times? > > forever?) but we'll never get a bulletproof test. >
Sign in to reply to this message.
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
