-
Notifications
You must be signed in to change notification settings - Fork 28
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
ENH(TEMP): disallow use of a known to be outdated version of dandi client #135
Conversation
…ient Only if DANDI_ALLOW_OUTDATED env variable is set, it will be just a good old warning. I wanted to provision some way to proceed in cases that some other issue (connectivity, OS, whatnot) prevents people from using dandi cli version checking
Codecov Report
@@ Coverage Diff @@
## master #135 +/- ##
==========================================
- Coverage 64.77% 64.72% -0.05%
==========================================
Files 37 37
Lines 2921 2923 +2
==========================================
Hits 1892 1892
- Misses 1029 1031 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
to test this, @satra, is there a way (env var, like |
don't have one at the moment (ET updates every 6 hours or so). i can manually delete the cache if needed. did not want to hand over cache refresh options to the world :) |
# or external issues by merely disclosing its availability to effected users, | ||
# while mandating users to update to the most recent version of the client | ||
if not bool(os.environ.get("DANDI_ALLOW_OUTDATED", None)): | ||
raise | ||
lgr.warning( | ||
"Failed to check for a more recent version available with etelemetry: %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this message seems more generic than perhaps we should use? you can use the flag raise_exception=True
in the call to check_available_version
and will return a RuntimeError
if a bad version is running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RuntimeError
? submitted sensein/etelemetry-client#23
as for raise_exception=True
- I should add it indeed but then, depending on etelemetry version I would not be reliably tell apart either it is outdated or some other exception has happened (there is sufficient amount of code outside of try/except in that function). I don't think we would like to stop on any exception etelemetry is throwing, but on that BadVersionException
- so I will wait on destiny of that PR
I think we better just stick to the "deployment/server specific" handling of versions (than overall one as would be possible with ET), so I will close this PR in favor of #186 |
Only if DANDI_ALLOW_OUTDATED env variable is set, it will be just a good old
warning. I wanted to provision some way to proceed in cases that some other
issue (connectivity, OS, whatnot) prevents people from using dandi cli version
checking
This is a band-aid toward upcoming "dandi-publish" RF until there is a way for a client to interrogate API about supported functionality.