Skip to content
This repository was archived by the owner on Apr 1, 2026. It is now read-only.

feat!: microgenerator changes - #203

Merged
kolea2 merged 35 commits into
googleapis:masterfrom
kolea2:microgenerator-changes
Feb 11, 2021
Merged

feat!: microgenerator changes#203
kolea2 merged 35 commits into
googleapis:masterfrom
kolea2:microgenerator-changes

Conversation

@kolea2

@kolea2kolea2 commented Feb 4, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

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 teamFebruary 4, 2021 15:57
@product-auto-labelproduct-auto-labelBot added the api: bigtable Issues related to the googleapis/python-bigtable API. label Feb 4, 2021
@google-clagoogle-claBot added the cla: yes This human has signed the Contributor License Agreement. label Feb 4, 2021
@snippet-bot

snippet-botBot commented Feb 4, 2021•
edited
Loading

Copy link
Copy Markdown

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

kolea2
kolea2 commented Feb 4, 2021
View reviewed changes
Comment threadgoogle/cloud/bigtable/column_family.py Outdated
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)

@kolea2kolea2Feb 4, 2021

Copy link
Copy Markdown
ContributorAuthor

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?

@crwilcoxcrwilcoxFeb 4, 2021

Copy link
Copy Markdown
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?

Comment threadgoogle/cloud/bigtable/table.py Outdated
Comment threadtests/unit/test_app_profile.py Outdated
Comment threadtests/unit/test_client.py
@kolea2
kolea2 requested a review from crwilcoxFebruary 4, 2021 16:03
kolea2
kolea2 commented Feb 4, 2021
View reviewed changes
Comment threadgoogle/cloud/bigtable/table.py Outdated
crwilcox
crwilcox reviewed Feb 5, 2021
View reviewed changes
Comment threadtests/unit/test_table.py Outdated
@kolea2kolea2 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 teamFebruary 10, 2021 20:52
kolea2
kolea2 commented Feb 10, 2021
View reviewed changes
Comment threadgoogle/cloud/bigtable/client.py
kolea2
kolea2 commented Feb 10, 2021
View reviewed changes
Comment threadgoogle/cloud/bigtable/table.py Outdated
if self.timeout is not None:
kwargs["timeout"] = timeout.ExponentialTimeout(deadline=self.timeout)

# todo confirm this change

@kolea2kolea2Feb 10, 2021

Copy link
Copy Markdown
ContributorAuthor

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

Comment threadtests/system.py
bigtable_table_admin_client_config as table_admin_config,
)

# from google.cloud.bigtable_admin_v2.gapic import (

@kolea2kolea2Feb 10, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@crwilcox ping

Comment threadtests/unit/test_table.py
bigtable_table_admin_pb2.ListBackupsRequest(
parent=parent, filter=backups_filter, **kwargs
),
retry=mock.ANY,

@kolea2kolea2Feb 10, 2021

Copy link
Copy Markdown
ContributorAuthor

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.

crwilcox
crwilcox approved these changes Feb 10, 2021
View reviewed changes
Comment threadgoogle/cloud/bigtable/client.py
Comment threadsamples/beam/requirements.txt Outdated
@kolea2kolea2 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:masterFeb 11, 2021
@kolea2
kolea2 deleted the microgenerator-changes branch February 11, 2021 15:12
@release-pleaserelease-pleaseBot mentioned this pull request Feb 11, 2021
Merged
@release-pleaserelease-pleaseBot mentioned this pull request Feb 25, 2021
Closed
@release-pleaserelease-pleaseBot mentioned this pull request Apr 6, 2021
Merged
tseaver added a commit that referenced this pull request Dec 8, 2021
@tseaver
fix: disable install-from-sorce for beam sample
b2813cf
Per #203.
parthea added a commit that referenced this pull request Jan 11, 2022
@parthea
add workaroud from PR#203
c133627
parthea added a commit that referenced this pull request Jan 11, 2022
@parthea
add workaroud from PR#203
54cddbb
parthea added a commit that referenced this pull request Jan 12, 2022
@tseaver@parthea@gcf-owl-bot
ci: run samples under Python 3.9 / 3.10 (#478)
58fa738
* 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>
@r-richmondr-richmond mentioned this pull request Feb 20, 2023
Merged
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@busunkim96busunkim96busunkim96 left review comments
@crwilcoxcrwilcoxcrwilcox approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: bigtableIssues related to the googleapis/python-bigtable API.cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kolea2@crwilcox@busunkim96