-
Couldn't load subscription status.
- Fork 834
fix: use tuples instead of packaging Version #1136
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
Conversation
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.
Ideally we wouldn't have this dependency, I just missed it during reviews. It is fairly basic parsing and I will look into replacing the package with just an inlined function, unless you would like to first of course!
592738c to
4d90571
Compare
|
@csmarchbanks removed packaging in favor of simple tuples, i think that should be enough. |
212c950 to
c2011ae
Compare
prometheus_client/utils.py
Outdated
| if not version_str: | ||
| return (0, 0, 0) | ||
| try: | ||
| return tuple(int(x) for x in version_str.split('.')) |
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.
I'm not sure if we have to expect some odd version here or if they are all normal and will convert to int.
882326f to
e0b9a60
Compare
Signed-off-by: Ruslan Kuprieiev <kupruser@gmail.com>
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.
Thanks! A couple things I think we should support just to be on the safe side:
- a
vprefix should be stripped off, it doesn't perfectly match the specs but seems silly to not recognize the version if it is present. - A build version, e.g. the third part has
-rc.0or something like that. Similarly, just strip it off the third part.
A test for a few different permutations of versions would be great as well
|
Just a friendly bump on my previous comments. I can also do the modifications myself if you do not have the time! |
Fixes #1135