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

Human readable message for Strava API rate limit #67

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

cs42
Copy link
Contributor

@cs42 cs42 commented Dec 12, 2023

Catch the Strava API rate limit exception and print a "human readable" message.

@martin-ueding
Copy link
Owner

Thank you for taking care of this!

I am not sure whether we need to exit there, though. The way it works now (as of 0.14.0), we download the time series for each activity before we add the activity metadata. Therefore all activities in the metadata should have a valid time series. I think it should be possible to just stop downloading activities and we could still display the ones that we got such that the user could start to play around.

So maybe a return instead of the sys.exit(1) would be possible? Otherwise this is “fail fast”, which I also like. I am not sure what would give a better user experience, currently slightly leaning to just having a return. What do you think?

@cs42
Copy link
Contributor Author

cs42 commented Dec 12, 2023

Could be a better solution, but exit was a much easier implementation and exceptions are not nice to read (further this exception is almost expected for a first run) ;-)

When implementing it without exit, we would need:

  1. a message in the web UI, that not all of data has been loaded because of the API rate limit
  2. ideally an automatic re-fetch after 15 minutes, else I think this would be even stranger for new users

@martin-ueding martin-ueding merged commit 4bfe18e into martin-ueding:master Dec 13, 2023
@martin-ueding
Copy link
Owner

I have implemented part of this now. The code retries automatically by waiting until the next quarter hour when the API limits are exceeded. The web server is only started after everything is downloaded.

Starting the webserver with a partial result while fetching more data in the background would require that we have some sort of updating the data structures while the server is running. Perhaps I will take a look at that later.

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.

None yet

2 participants