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

feat!: microgenerator changes #203

Merged
merged 35 commits into from Feb 11, 2021
Merged

Conversation

kolea2
Copy link
Collaborator

@kolea2 kolea2 commented Feb 4, 2021

Release-As: v2.0.0-dev1

Regen the client with the microgenerator. This requires the client to be used with Python >= 3.6

There are a few outstanding questions I have that I will annotate in the PR. Most important changes to pay attention to are the handwritten client and test layers.

@kolea2 kolea2 requested a review from a team as a code owner February 4, 2021 15:57
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/python-bigtable API. label Feb 4, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 4, 2021
@snippet-bot
Copy link

snippet-bot bot commented Feb 4, 2021

Here is the summary of changes.

You added 1 region tag.
You deleted 1 region tag.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

max_age = _helpers._duration_pb_to_timedelta(gc_rule_pb.max_age)
return MaxAgeGCRule(max_age)
# todo check this is right
# max_age = _helpers._duration_pb_to_timedelta(gc_rule_pb.max_age)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

question - is this change the correct new approach?

Copy link
Contributor

Choose a reason for hiding this comment

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

This might be good to dig into. protoplus knows timestamps, and I would expect gc_rule_pb.max_age to be one, without the helper? Though timedelta isn't timestamp and I don't recall offhand if that is implemented. to be safe, I'd expect gc_rule_pb._pb.max_age to allow the other code to stay the same?

google/cloud/bigtable/table.py Outdated Show resolved Hide resolved
tests/unit/test_app_profile.py Outdated Show resolved Hide resolved
tests/unit/test_client.py Show resolved Hide resolved
@kolea2 kolea2 requested a review from crwilcox February 4, 2021 16:03
google/cloud/bigtable/table.py Outdated Show resolved Hide resolved
tests/unit/test_table.py Outdated Show resolved Hide resolved
@kolea2 kolea2 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 8, 2021
@kolea2 kolea2 requested a review from a team as a code owner February 10, 2021 20:52
@@ -1071,10 +1087,11 @@ def _do_mutate_retryable_rows(self):
if self.timeout is not None:
kwargs["timeout"] = timeout.ExponentialTimeout(deadline=self.timeout)

# todo confirm this change
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@crwilcox another todo I missed - I think my concern here was the mutate rows call wasn't wrapped in a request={... but I think due to the code below this (will need to expand) this works as expected. LMK if you have a concern

bigtable_table_admin_client_config as table_admin_config,
)

# from google.cloud.bigtable_admin_v2.gapic import (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@crwilcox ping

bigtable_table_admin_pb2.ListBackupsRequest(
parent=parent, filter=backups_filter, **kwargs
),
retry=mock.ANY,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@crwilcox one last thing to point out - the last few params were now removed from the assertion because we are no longer using the inner_api_calls to get the result, let me know if oyu think this should still be here.

google/cloud/bigtable/client.py Show resolved Hide resolved
samples/beam/requirements.txt Outdated Show resolved Hide resolved
@kolea2 kolea2 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 11, 2021
@kolea2 kolea2 merged commit b31bd87 into googleapis:master Feb 11, 2021
@kolea2 kolea2 deleted the microgenerator-changes branch February 11, 2021 15:12
@release-please release-please bot mentioned this pull request Apr 6, 2021
tseaver added a commit that referenced this pull request Dec 8, 2021
parthea added a commit that referenced this pull request Jan 11, 2022
parthea added a commit that referenced this pull request Jan 11, 2022
parthea added a commit that referenced this pull request Jan 12, 2022
* ci: run samples under Python 3.9 / 3.10

Refresh each sample's noxfile via:

----------------------------- %< -----------------------------
$ for noxfile in samples/*/noxfile.py; do
    echo "Refreshing $noxfile";
    wget -O $noxfile https://github.com/GoogleCloudPlatform/python-docs-samples/raw/main/noxfile-template.py
    echo "Blackening samples for $noxfile"
    nox -f $noxfile -s blacken
done
----------------------------- %< -----------------------------

Closes #477.

* fix: disable install-from-sorce for beam sample

Per #203.

* fix: skip beam sample for Python 3.10

Beam-related wheels are not yet available.

* fix: also refresh noxfiles for 'samples/snippets'

* ci: don't enforce type hints on old samples

* resolve issue where samples templates are not updated

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* resolve mypy error Name __path__ already defined

* add workaroud from PR #203

Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
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 googleapis/python-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.

None yet

3 participants