Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

chore: transition the library to microgenerator #56

Merged
merged 10 commits into from Sep 21, 2020

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Sep 16, 2020

Closes #41.

Remaining things:

  • Add the UPGRADING guide to docs.

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 16, 2020
@product-auto-label product-auto-label bot added the api: bigquerydatatransfer Issues related to the googleapis/python-bigquery-datatransfer API. label Sep 17, 2020
@plamut
Copy link
Contributor Author

plamut commented Sep 17, 2020

I see potential issues with namespace clashes.

The following imports used with the old generated client do not work anymore:

from google.cloud import bigquery_datatransfer
from google.cloud import bigquery_datatransfer_v1

These imports need to be changed to the following:

from google.cloud.bigquery import datatransfer
from google.cloud.bigquery import datatransfer_v1

This is all well and good, but if one then installs google-cloud-bigquery, these imports stop working:

$ pip install google-cloud-bigquery
$ python3.8
...
>>> from google.cloud.bigquery import datatransfer
ImportError: cannot import name 'datatransfer' from 'google.cloud.bigquery'

Uninstalling google-cloud-bigquery makes the datatransfer import work again.

@busunkim96 Do you maybe know if any other 100% autogenerated libraries experienced the same problem and how they fixed it?

Edit:
And just when I typed the word "namespace", I got an idea that worked - google.cloud.bigquery needs to be added to namespace packages in setup.py:

namespaces.append("google.cloud.bigquery")

The generated code tests do not cover all code paths after all...
@plamut plamut marked this pull request as ready for review September 17, 2020 14:10
@plamut plamut requested a review from a team September 17, 2020 14:10
@plamut plamut requested a review from a team as a code owner September 17, 2020 14:10
@plamut plamut requested review from kurtisvg, danoscarmike, busunkim96, tswast and a team and removed request for a team and kurtisvg September 17, 2020 14:10
@busunkim96
Copy link
Contributor

@plamut @tswast If you all prefer to retain the old namespacing you can do that via generator args

https://github.com/googleapis/googleapis/blob/b821f320473c8ec05a1c7fb9a496c958b1ab9834/google/devtools/clouderrorreporting/v1beta1/BUILD.bazel#L161

opt_args = ["python-gapic-namespace=google.cloud", "python-gapic-name=bigquery_datatransfer"],

Not sure where we landed with the other BigQuery libraries though, I assume it's better for them to be consistent?

@danoscarmike danoscarmike removed their request for review September 17, 2020 15:58
@danoscarmike
Copy link
Contributor

Thanks @plamut! I'll defer to @busunkim96 and @tswast on this one.

@tswast
Copy link
Contributor

tswast commented Sep 17, 2020

Not sure where we landed with the other BigQuery libraries though, I assume it's better for them to be consistent?

https://github.com/googleapis/python-bigquery-reservation and https://github.com/googleapis/python-bigquery-connection both use the google.cloud.bigquery namespace. (They might actually not be working properly, as I haven't tested them yet)

@plamut
Copy link
Contributor Author

plamut commented Sep 17, 2020

OK, I'll keep the google.cloud.bigquery namespace then and document it in the UPGRADING guide accordingly.

@plamut
Copy link
Contributor Author

plamut commented Sep 18, 2020

Added the UPGRADING guide and simplified the sample, the PR should be done now.

After approving and merging, I will follow up with a release PR (probably on Monday).

UPGRADING.md Outdated Show resolved Hide resolved

# ----------------------------------------------------------------------------
# Samples templates
# ----------------------------------------------------------------------------

python.py_samples(skip_readmes=True)

# Fix missing async client in datatransfer_v1
s.replace(
Copy link
Contributor

Choose a reason for hiding this comment

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

The generator isn't adding the async client to __init__.py?

Copy link
Contributor Author

@plamut plamut Sep 21, 2020

Choose a reason for hiding this comment

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

IIRC something tried to import an Async client somewhere, but it did not exist, thus added this import.

Edit: Misunderstood. Yes, that import was missing, thus added it manually. I probably forgot to open an issue?

Edit 2: This was to make sure that datatransfer and datatransfer_v1 are the same (one lacked the async client in __all__).

Co-authored-by: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>
@plamut plamut merged commit ad6d893 into googleapis:master Sep 21, 2020
@plamut plamut deleted the use-microgenerator branch September 24, 2020 09:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: bigquerydatatransfer Issues related to the googleapis/python-bigquery-datatransfer API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transition the library to the new microgenerator
4 participants