-
Notifications
You must be signed in to change notification settings - Fork 99
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
chore: dont rely on slice labels #184
chore: dont rely on slice labels #184
Conversation
d63b908
to
af48549
Compare
@MichaHoffmann can you give me a bit more context? How is this code affecting external behavior in the Thanos build? |
@squat There is the new "stringlabels" code in Prometheus itself. When switching to this implementation, accessing the slice directly is no longer allowed. |
Here's what you get if you build prom-label-proxy now:
With this change, the build is successful. |
What @SuperQ said! Just adding context that I want to convert Thanos to be compatible with the stringlabels tag and there are more build failures, this was just one of them and it seemed easy enough to fix. |
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 a maintainer here but had a comment if efficiency is a concern.
Signed-off-by: Michael Hoffmann <[email protected]>
af48549
to
2bd8cbc
Compare
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 for the context @SuperQ @MichaHoffmann 🏄♀️🏄♀️🏄♀️
And soon |
Internal change for library compatibility. No user-visible changes. * [CHANGE] Don't rely on slice labels #184 Signed-off-by: SuperQ <[email protected]>
Internal change for library compatibility. No user-visible changes. * [CHANGE] Don't rely on slice labels #184 Signed-off-by: SuperQ <[email protected]>
I wanted to proceed a bit on trying to convert thanos to build with stringlabels and prom-label-proxy is used as a library there. This would fix this build error for me!