Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safe refresh #32

Closed
wants to merge 8 commits into from
Closed

Safe refresh #32

wants to merge 8 commits into from

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!

@@ -13,6 +13,9 @@ class UserFitbit(models.Model):
access_token = models.TextField()
auth_secret = models.TextField()
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.

3 participants