Control BTF authentication via env. variables; cache part of BTF API; resolves #60 #66
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- remove passing around of session variable - use time difference to determine if a new auth is required - allow forcing of new auth (e.g. when getting an 401 HTTP)
davidverweij
added
bug
Closed
|
NOTE, a few things are left to do; for which I have created Issue #67 |
jawrainey
requested changes
Apr 6, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR, I've restructured BTF's authentication from being a class attribute, to being stored in the environmental variables; much like how this is done at the
dmpypackage. This will ensure that the authentication is re-done every 50 minutes (10 minutes before a token expires) - this check is triggered when preparing for an actual GET request. The proposed solution moved the authentication to thelib/byteflies.pyand thereby also removes the need to pass arequests.Sessionvariable around.Additionally, I've cached the
__get_recording_by_id()(maxsize of 1). Since we wanted the code to run for any individual file, at any time (no dependencies on living variables), the pipeline made subsequent calls to the exact same API endpoint. With the proposed changes, the result of this call is cached for max 5 minutes (since the links in the payload are valid for only 10). A quick test shows (for example) out of 21 files to be downloaded from one recording, 7 files to be downloaded were based on the same API call and have now been reduced to one. The other 14 calls could not be cached as they call a deeper API link to retrieve the algorithm download url; which has to be done for each algorithm file. I've turned down thesleep.time()from 1 to .5 to speed this up a bit.Testing
poetry run nox -rpoetry run consumerpython data_transfer/main.py BTF Muenster 0 3Any feedback highly appreciated @jawrainey @ColinBD !