Skip to content

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Sep 11, 2025

Fixes #1135

@efiop
Copy link
Contributor Author

efiop commented Sep 11, 2025

Copy link
Member

@csmarchbanks csmarchbanks left a 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!

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

efiop commented Sep 11, 2025

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

@efiop efiop force-pushed the patch-1 branch 2 times, most recently from 212c950 to c2011ae Compare September 11, 2025 22:19
@efiop efiop requested a review from csmarchbanks September 11, 2025 22:23
if not version_str:
return (0, 0, 0)
try:
return tuple(int(x) for x in version_str.split('.'))
Copy link
Contributor Author

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 efiop force-pushed the patch-1 branch 3 times, most recently from 882326f to e0b9a60 Compare September 11, 2025 22:42
Signed-off-by: Ruslan Kuprieiev <kupruser@gmail.com>
Copy link
Member

@csmarchbanks csmarchbanks left a 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:

  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
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:master Sep 18, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants