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
updating prometheus/common to v0.25.0 from v0.10.0 #102144
Conversation
Signed-off-by: RinkiyaKeDad <arshsharma461@gmail.com>
Hi @RinkiyaKeDad. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: RinkiyaKeDad The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/ok-to-test |
The |
github.com/Shopify/sarama => github.com/Shopify/sarama v1.19.0 | ||
github.com/Shopify/toxiproxy => github.com/Shopify/toxiproxy v2.1.4+incompatible | ||
github.com/VividCortex/gohistogram => github.com/VividCortex/gohistogram v1.0.0 | ||
github.com/afex/hystrix-go => github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5 |
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.
what brought all of these in?
github.com/asaskevich/govalidator => github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a | ||
github.com/auth0/go-jwt-middleware => github.com/auth0/go-jwt-middleware v0.0.0-20170425171159-5493cabe49f7 | ||
github.com/aws/aws-lambda-go => github.com/aws/aws-lambda-go v1.13.3 |
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.
what brought in new cloud provider libs?
github.com/aws/aws-sdk-go => github.com/aws/aws-sdk-go v1.35.24 | ||
github.com/aws/aws-sdk-go-v2 => github.com/aws/aws-sdk-go-v2 v0.18.0 |
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.
what brought in new cloud provider libs?
github.com/ianlancetaylor/demangle => github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6 | ||
github.com/imdario/mergo => github.com/imdario/mergo v0.3.5 | ||
github.com/inconshreveable/mousetrap => github.com/inconshreveable/mousetrap v1.0.0 | ||
github.com/influxdata/influxdb1-client => github.com/influxdata/influxdb1-client v0.0.0-20191209144304-8bf82d3c094d |
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.
what brought in a db client dependency?
This looks to have brought in a startling number of additional dependencies, and is the sort of thing we want to catch/prevent with our dependency checking. Use |
@RinkiyaKeDad: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@liggitt found the source of the boatload of deps - prometheus/common#255 |
@RinkiyaKeDad: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
2 similar comments
@RinkiyaKeDad: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@RinkiyaKeDad: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
that's exceptionally unfortunate, especially since the library doesn't seem concerned about the issue |
@RinkiyaKeDad: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@RinkiyaKeDad given that promethus/common and go-kit folks will take their own time given comments in prometheus/common#255 and prometheus/common#302 Let's not waste our time. If it happens we can update then. |
Closing till we figure out a way to avoid the extra dependencies which come with github.com/go-kit/kit |
The indirect dependencies appear in your go.mod, but unless you use them, they aren't brought into the build, and they don't make it into any artifacts. I completely understand the desire to keep dependency graphs small, but there is limited effect on the module, and zero effect on any artifacts. And for a project like this one, that particular ship sailed a long time ago — the marginal cost of the additional deps brought in by Go kit is practically zero. |
@peterbourgon Ack and thanks! we are still trying and it's slowly getting better with every release. |
github.com/go-kit/kit is only being used for the log packages and brings in a lot of dependencies as compared to github.com/go-kit/log. This would help determine how useful switching would be. For reference see kubernetes/kubernetes#102144 and prometheus/common#255 (comment)
Signed-off-by: RinkiyaKeDad arshsharma461@gmail.com
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Updates github.com/prometheus/common from v0.10.0 to v0.25.0.
Special notes for your reviewer:
Final output from
hack/update-vendor.sh
had this as the last line:Does something needs to be done in regards to this?
Does this PR introduce a user-facing change?
Link to the relevant slack discussion.
/assign @dims