Skip to content

fix: use tuples instead of packaging Version - #1136

Merged
csmarchbanks merged 1 commit into
prometheus:masterfrom
efiop:patch-1
Sep 18, 2025
Merged

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

Conversation

@efiop

Copy link
Copy Markdown
Contributor

Fixes#1135

@efiop

Copy link
Copy Markdown
ContributorAuthor

CC @csmarchbanks

@csmarchbankscsmarchbanks 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.

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!

@efiopefiop changed the title fix: don't forget packaging as a dependencyfix: use tuples instead of packaging VersionSep 11, 2025
@efiop
efiopforce-pushed the patch-1 branch 4 times, most recently from 592738c to 4d90571CompareSeptember 11, 2025 22:15
@efiop

Copy link
Copy Markdown
ContributorAuthor

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

@efiop
efiopforce-pushed the patch-1 branch 2 times, most recently from 212c950 to c2011aeCompareSeptember 11, 2025 22:19
Comment threadprometheus_client/utils.py Outdated
if not version_str:
return (0, 0, 0)
try:
return tuple(int(x) for x in version_str.split('.'))

Copy link
Copy Markdown
ContributorAuthor

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.

@efiop
efiopforce-pushed the patch-1 branch 3 times, most recently from 882326f to e0b9a60CompareSeptember 11, 2025 22:42
Signed-off-by: Ruslan Kuprieiev <kupruser@gmail.com>

@csmarchbankscsmarchbanks 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.

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

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
csmarchbanks merged commit 266beb2 into prometheus:masterSep 18, 2025
11 checks passed
Sign up for freeto 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.

0.23.0 is using packaging but not declaring it as a dependency

2 participants

@efiop@csmarchbanks