Skip to content

Conversation

@brad
Copy link
Member

@brad brad commented Apr 22, 2016

@orcasgit/orcas-developers This has turned out to be a bit of a refactor, but I think refreshing tokens works better now. The changes:

  • New expires_at and timezone fields
  • More careful updating of saved tokens when refreshed. We check that the new expires_at is greater than the old and that the expires_at time is greater than now.
  • Added the transaction.atomic() decorator to the get_fitbit_data method.
  • more/better error handling for celery tasks. I think it's set up now so that every call to the API handles rate limit exceptions gracefully
  • Use timezone from the UserFitbit object rather than dealing with the session
  • PEP8 updates

@coveralls
Copy link

Coverage Status

Coverage decreased (-7.9%) to 85.75% when pulling 802d7a0 on safe-refresh into ae6b524 on master.

field=models.CharField(default='UTC', max_length=128),
preserve_default=False,
),
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brad What do you think about renaming this file to 0006_add_expires_timezone_fields.py?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@percyperez Oh yeah, of course!

refresh_token = models.TextField()
# We will store the timestamp float number as it comes from Fitbit here
expires_at = models.FloatField()
timezone = models.CharField(max_length=128)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brad I see that the migration file for the timezone field has default='UTC'. Did you remove default='UTC' here after the migration file was created? If not, how default='UTC' was added to the migration file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@percyperez When I ran makemigrations it asked for a default and I specifie UTC

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brad Ah Cool 👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-7.9%) to 85.75% when pulling 89df0b5 on safe-refresh into ae6b524 on master.



@shared_task
def update_user_timezone(fitbit_user):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@brad
Copy link
Member Author

brad commented Apr 25, 2016

Superseded by #33

@brad brad closed this Apr 25, 2016
@brad brad deleted the safe-refresh branch April 25, 2016 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants