Skip to content
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

bigtable: Update user-agent to the latest version #3429

Merged
merged 2 commits into from
Dec 17, 2020

Conversation

mohanli-ml
Copy link
Contributor

@mohanli-ml mohanli-ml commented Dec 8, 2020

Currently the user agent of Bigtable in Go is standalone, as in https://github.com/googleapis/google-cloud-go/blob/master/bigtable/doc.go#L121m. Update it to the latest version https://pkg.go.dev/cloud.google.com/go/bigtable. Related issue: #3330.

Background: DirectPath will use user agent to deny a specific library version, so we want to update the user agent with the library release date.

@mohanli-ml mohanli-ml requested review from tritone and a team as code owners December 8, 2020 23:59
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 8, 2020
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Dec 8, 2020
Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@igorbernstein2 @tbpg is there any reason we can't either 1. Use

const Repo = "20201104"
for this, which is used by at least some other packages, or 2. Switch to using the release tag itself?

It's easy to forget to update this value as is (as evidenced by the fact that it hasn't been updated in 2.5 years).

@codyoss
Copy link
Member

codyoss commented Dec 9, 2020

@tritone I will be changing this soon to be the actual tag as a part of #2749.

@codyoss
Copy link
Member

codyoss commented Dec 9, 2020

@mohanli-ml Won't direct path be denied in older version automatically since the new option is not being sent, why is extra header validation needed? Also, like you mention in #3330 ideally this would be semver. If we switch this header to be semver you would no longer be able to parse a date for this feature.

@mohanli-ml
Copy link
Contributor Author

mohanli-ml commented Dec 9, 2020

@codyoss Yes, older versions before directpath release will not support directpath. However, we need this user agent for future. For example, we might have 3 versions that can support DP, and we realize one version has some problem, so we want to deny that one, but leave the other two still use DP.

For semver, both semver and date can work, as long as the user agent is compatible with the library version.

Since this is a blocker for me, may I ask do you have an ETA for this? How about I change the user agent to semver, so that you can approve it? Thanks!

@codyoss
Copy link
Member

codyoss commented Dec 15, 2020

@tritone Is this still waiting on anything? My comments were just that and don't want to hold anything up here.

@tritone
Copy link
Contributor

tritone commented Dec 15, 2020

I'm fine with the switch to use the semver here. @igorbernstein2 @kolea2 can you confirm that this won't cause any backend or data collection issues that you know of?

@tritone tritone added the automerge Merge the pull request once unit tests and other checks pass. label Dec 17, 2020
@gcf-merge-on-green gcf-merge-on-green bot merged commit 81fc2d1 into googleapis:master Dec 17, 2020
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants