-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: send kube version in user-agent of aiven API clients #655
Conversation
1bffbd4
to
c25efbb
Compare
c25efbb
to
573cd18
Compare
573cd18
to
8a6a2dc
Compare
93f486d
to
ad6a2bc
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.
LGTM, although now we have two places where operatorVersion
is specified. It would be good to have a single source of truth.
https://github.com/aiven/aiven-operator/pull/655/files#diff-96e66e1ccb0083ac5a2b7fc37cb002803d242ea67c1981a5d09ef8ee2668354cR38-R40 + https://github.com/aiven/aiven-operator/pull/655/files#diff-2873f79a86c0d8b3335cd7731b0ecf7dd4301eb19a82ef7a1cba7589b5252261R32-R34
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.
As @rriski mentioned, we need to get rid of the additional definition of the provider version.
We also should make the approach similar to the one in our Terraform Provider, here's how it works:
version
is defined somewhere, preferably inmain.go
.- It is then overwritten during the compile time via
go build
'sldflags
parameter, similarly to this approach.
TL;DR. While I think it's unreasonable to add GoReleaser support in this PR, I think it'll be beneficial for us to define the meaningful operatorVersion
in one place only; this could be done by setting the one in suite_test.go
to always be a constant string test
, like we do over here. This also allows to understand, via the User-Agent
, which requests originated from our tests, and which haven't.
ad6a2bc
to
f8b37db
Compare
Co-authored-by: Jeff Held <[email protected]>
f8b37db
to
a6fab25
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.
lgtm
No description provided.