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

v3: add migration helpers and guide for breaking changes #960

Closed
tswast opened this issue Sep 10, 2021 · 5 comments
Closed

v3: add migration helpers and guide for breaking changes #960

tswast opened this issue Sep 10, 2021 · 5 comments
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. next major: breaking change this is a change that we should wait to bundle into the next major version semver: major Hint for users that this is an API breaking change. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@tswast
Copy link
Contributor

tswast commented Sep 10, 2021

I'd like to make sure we don't get folks stuck on v2. I know we have some internal (Google) customers who rely on the BQML stats protobuf messages (the ones in google.cloud.bigquery_v2.

I propose

  1. We restore the files in google.cloud.bigquery_v2, but document that they are deprecated and are not updated.
  2. We continue to return dictionaries in the relevant classes, so that all stays the same.
  3. We document in the UPGRADING guide how to manually convert from the dictionaries to the classes in google.cloud.bigquery_v2 with some code samples.

Regarding the pandas connector changes, we should also document those in the UPGRADING guide, ideally with code samples on how to get / convert to the old dtypes if desired.

@tswast tswast added api: bigquery Issues related to the googleapis/python-bigquery API. next major: breaking change this is a change that we should wait to bundle into the next major version type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Sep 10, 2021
@tswast
Copy link
Contributor Author

tswast commented Sep 10, 2021

@plamut Does this make sense? I want to keep the changes we've made and avoid using/updating the bigquery_v2 code in the client, but the type classes can still be available so that folks can migrate to dictionary-style access at their own pace.

@tswast tswast added the semver: major Hint for users that this is an API breaking change. label Sep 10, 2021
@plamut
Copy link
Contributor

plamut commented Sep 13, 2021

We restore the files in google.cloud.bigquery_v2, but document that they are deprecated and are not updated.

How is that supposed to work, just keeping those type classes around so that one can import them if really needed? But otherwise no compatibility logic and no direct exposure in the top level namespace google.cloud.bigquery? In other words, the users will be entirely responsible for converting these classes to/from dicts?

Re: conversions - we'll probably recommend protobuf.json_format for that, and presumably users will have to install that dependency manually?

With these assumptions, I think it could work fine with minimal overhead, yes. As a bonus, we could also issue a runtime warning if anything is imported from the bigquery_v2 namespace.

Edit: Will we also keep the v2 type classes in the reference docs? Noticed this aspect when resolving a merge conflict on the v2 removal PR.

@tswast
Copy link
Contributor Author

tswast commented Sep 13, 2021

That's right. Just keep the autogenerated classes around and show folks how to parse JSON. Probably with a code sample for what we used to do that ignored unknown enums and such, especially since we don't plan to keep the type classes updated.

As a bonus, we could also issue a runtime warning if anything is imported from the bigquery_v2 namespace.

That's a great idea.

Edit: Will we also keep the v2 type classes in the reference docs? Noticed this aspect when resolving a merge conflict on the v2 removal PR.

Yes, let's keep them in the docs and add a deprecation warning.

@plamut
Copy link
Contributor

plamut commented Sep 15, 2021

Looking at #855 and how large it is, it IMO makes sense to do this in a separate PR after that large PR has been merged.

@meredithslota
Copy link
Contributor

Closed in #888 and #1027.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. next major: breaking change this is a change that we should wait to bundle into the next major version semver: major Hint for users that this is an API breaking change. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants