FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: use tuples instead of packaging Version by efiop · Pull Request #1136 · prometheus/client_python · GitHub

/ client_python Public

fix: use tuples instead of packaging Version - #1136

Merged
csmarchbanks merged 1 commit into
prometheus:masterfrom
efiop:patch-1
Sep 18, 2025
ConversationCommits (1)ChecksFiles changed
Merged

fix: use tuples instead of packaging Version#1136
csmarchbanks merged 1 commit into
prometheus:masterfrom
efiop:patch-1

Conversation

efiop commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

Fixes #1135

efiop commented Sep 11, 2025

Copy link
Copy Markdown
Contributor Author

CC @csmarchbanks

efiop mentioned this pull request Sep 11, 2025
Closed
csmarchbanks reviewed Sep 11, 2025
View reviewed changes

csmarchbanks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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!

efiop changed the title fix: don't forget packaging as a dependency fix: use tuples instead of packaging Version Sep 11, 2025
efiop force-pushed the patch-1 branch 4 times, most recently from 592738c to 4d90571 Compare September 11, 2025 22:15

efiop commented Sep 11, 2025

Copy link
Copy Markdown
Contributor Author

@csmarchbanks removed packaging in favor of simple tuples, i think that should be enough.

efiop force-pushed the patch-1 branch 2 times, most recently from 212c950 to c2011ae Compare September 11, 2025 22:19
efiop requested a review from csmarchbanks September 11, 2025 22:23
efiop commented Sep 11, 2025
View reviewed changes
Comment thread prometheus_client/utils.py Outdated
if not version_str:
return (0, 0, 0)
try:
return tuple(int(x) for x in version_str.split('.'))

efiop Sep 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm not sure if we have to expect some odd version here or if they are all normal and will convert to int.

efiop force-pushed the patch-1 branch 3 times, most recently from 882326f to e0b9a60 Compare September 11, 2025 22:42
fix: use tuples instead of packaging Version
ebfc789 
Signed-off-by: Ruslan Kuprieiev <kupruser@gmail.com>
csmarchbanks reviewed Sep 12, 2025
View reviewed changes

csmarchbanks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks! A couple things I think we should support just to be on the safe side:

  1. a v prefix should be stripped off, it doesn't perfectly match the specs but seems silly to not recognize the version if it is present.
  2. A build version, e.g. the third part has -rc.0 or 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

csmarchbanks commented Sep 16, 2025

Copy link
Copy Markdown
Member

Just a friendly bump on my previous comments. I can also do the modifications myself if you do not have the time!

csmarchbanks merged commit 266beb2 into prometheus:master Sep 18, 2025
11 checks passed
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

csmarchbanks csmarchbanks approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

0.23.0 is using packaging but not declaring it as a dependency

2 participants

Back | FazBrowse Home | New Git URL