patternpythondjangoMinor
Storing folders of list of records in Google Drive
Viewed 0 times
googlerecordsfoldersdriveliststoring
Problem
I've written a script using the Django ORM to store 'configuration settings' and interacting with the Google Drive API and another API. The script creates a base project folder and subfolders in Google Drive for a list of records, and posts links back to the source database. I'm a beginner in Python, so I'm not sure if I'm doing things the right way, or if it's completely terrible.
I think there are some areas that could be cleaned up -- for example, how I am creating the 'client' and 'service' objects repeatedly inside specific methods. I wasn't sure if it would work to create the objects and pass them between the methods though. I think I could also use the Google 'batch' and 'partial response' methods to optimize HTTP requests.
Also, I need to get this thing on a periodic task or crontab. I was interested in trying celery, so I initially set up the below jobs, and they were creating the clients, but I don't think it's working anymore in Celery.
I removed some of the script and commented areas to make it more understandable.
```
# Celery tasks
@periodic_task(run_every=crontab(hour="", minute="", day_of_week="*"))
def sync_folders():
configs = Configuration.objects.all()
for config in configs:
user = config.user
client = create_client(user, config)
try:
create_document_folders(config, client)
except Exception as inst:
pass
@periodic_task(run_every=crontab(hour="", minute="5", day_of_week=""))
def sync_files():
configs = Configuration.objects.all()
for config in configs:
user = config.user
client = create_client(user, config)
try:
get_updated_files(config, client)
except Exception as inst:
pass
# Rest of post is the script initiated by celery tasks above.
def create_client(user, config):
# Create the QuickBase Client object
s = config
if s is not None:
if s.qb_realm is not None:
if s.qb_realm != 'w
I think there are some areas that could be cleaned up -- for example, how I am creating the 'client' and 'service' objects repeatedly inside specific methods. I wasn't sure if it would work to create the objects and pass them between the methods though. I think I could also use the Google 'batch' and 'partial response' methods to optimize HTTP requests.
Also, I need to get this thing on a periodic task or crontab. I was interested in trying celery, so I initially set up the below jobs, and they were creating the clients, but I don't think it's working anymore in Celery.
I removed some of the script and commented areas to make it more understandable.
```
# Celery tasks
@periodic_task(run_every=crontab(hour="", minute="", day_of_week="*"))
def sync_folders():
configs = Configuration.objects.all()
for config in configs:
user = config.user
client = create_client(user, config)
try:
create_document_folders(config, client)
except Exception as inst:
pass
@periodic_task(run_every=crontab(hour="", minute="5", day_of_week=""))
def sync_files():
configs = Configuration.objects.all()
for config in configs:
user = config.user
client = create_client(user, config)
try:
get_updated_files(config, client)
except Exception as inst:
pass
# Rest of post is the script initiated by celery tasks above.
def create_client(user, config):
# Create the QuickBase Client object
s = config
if s is not None:
if s.qb_realm is not None:
if s.qb_realm != 'w
Solution
# Celery tasks
@periodic_task(run_every=crontab(hour="*", minute="*", day_of_week="*"))
def sync_folders():
configs = Configuration.objects.all()
for config in configs:
user = config.user
client = create_client(user, config)I'd do:
create_client(config.user, config)try:
create_document_folders(config, client)
except Exception as inst:
passDon't do this. You catch an exception and ignore it. You have no clue what went wrong. You should at the very least print out the exception, or put it in a log, or something so you can figure out what what goes wrong.
@periodic_task(run_every=crontab(hour="*", minute="5", day_of_week="*"))
def sync_files():
configs = Configuration.objects.all()
for config in configs:
user = config.user
client = create_client(user, config)
try:
get_updated_files(config, client)
except Exception as inst:
passPretty the same function again. Refactor them and pass
get_updated_files or create_document_folder as a parameter to the new function.# Rest of post is the script initiated by celery tasks above.
def create_client(user, config):
# Create the QuickBase Client object
s = configWhy? Just use config. The only advantage to s is that it's harder to read.
if s is not None:Do you really want to support config being None?
if s.qb_realm is not None:
if s.qb_realm != 'www':
baseurl = 'https://' + s.qb_realm + '.quickbase.com'Generally its better to use string formatting than concatenation.
else:
baseurl = 'https://www.quickbase.com'Why is this a special case? it just does the same thing that the above case would do.
else:
baseurl = 'https://www.quickbase.com'
client = quickbase.Client(QUICKBASE_USER, QUICKBASE_PASSWORD, base_url=baseurl)
return clientCombine those two lines
def get_jobs_wo_folders(config, client):
if client is not None:Having a function silently fail to do its job given bad input is a bad idea. You should throw an exception. I wouldn't even bother to check if client were None, and just let it fail when it tries to use it.
query = "{'" + config.root_folder_id + "'.EX.''}"
clist = [config.root_folder_id, config.root_folder_name_fid, config.root_folder_user, '3']
records = client.do_query(query, columns=clist, database=config.qb_root_dbid)
return records
def retrieve_specific_files(service, param):
# Search and retrieve a list of file resources passing a params object
result = []
page_token = None
while True:
try:
if page_token:Use
is not None to check for None, just because a few other random things end up being false and its more explicit this wayparam['pageToken'] = page_token
files = service.files().list(**param).execute()
result.extend(files['items'])
page_token = files.get('nextPageToken')
if not page_token:
break
except errors.HttpError, error:
print 'An error occurred: %s' % error
breakDo you really want to try and continue on if there is an error? Shouldn't you just give up this attempt?
return result
def get_csv(list):avoid list as a variable name since its a builtin python type
si = cStringIO.StringIO()
cw = csv.writer(si)
cw.writerows(list)
return si.getvalue().strip('\r\n')
# Google helper functions
def refresh_access_token(user, config):
instance = UserSocialAuth.objects.get(user=user, provider='google-oauth2')
instance.refresh_token()
# Configuration model has a field 'last_refresh_time' that is auto-now date/time
config.save()
return instance.extra_data['access_token']
def get_access_token(config):
try:
access_token = UserSocialAuth.objects.get(user=config.user, provider='google-oauth2').extra_data['access_token']You just did something very similiar, this suggests that you should extract a common function
return access_token
except Exception as inst:
pprint(inst)At least here you print the exception.
def create_drive_service(config):
"""
Creates a drive service using the 'Configuration' object's
related user. Users initial signup is done with django-socialauth;
the access token and refresh token is saved with django-socialauth &
refresh token handling is done with django-socialauth.
"""
c = config
user = config.userWhy do you sometimes use
config.user and other times pass the user as a parameter?instance = UserSocialAuth.objects.get(user=user, provider='google-oauth2')avoid generic names like instance
```
refreshed_at = c.token_last_refresh
now = datetime.now()
expires_in = instance.extra_data['expires']
token_age = (now - refreshed_at).seconds
Code Snippets
# Celery tasks
@periodic_task(run_every=crontab(hour="*", minute="*", day_of_week="*"))
def sync_folders():
configs = Configuration.objects.all()
for config in configs:
user = config.user
client = create_client(user, config)try:
create_document_folders(config, client)
except Exception as inst:
pass@periodic_task(run_every=crontab(hour="*", minute="5", day_of_week="*"))
def sync_files():
configs = Configuration.objects.all()
for config in configs:
user = config.user
client = create_client(user, config)
try:
get_updated_files(config, client)
except Exception as inst:
pass# Rest of post is the script initiated by celery tasks above.
def create_client(user, config):
# Create the QuickBase Client object
s = configif s is not None:Context
StackExchange Code Review Q#21597, answer score: 2
Revisions (0)
No revisions yet.