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.

Sign up for GitHub

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

Jump to bottom

feat!: migrate to v2.0.0 #147

Merged
merged 99 commits into from
Nov 11, 2020
Merged

feat!: migrate to v2.0.0 #147

merged 99 commits into from
Nov 11, 2020

Conversation

larkee
Copy link
Contributor

@larkee larkee commented Oct 2, 2020 •
edited
Loading

This PR is a preview of the 2.0 release.

TODO list:

  • Fix docs.

  • Describe changes in UPGRADING guide.

BREAKING CHANGES

  • list_instances, list_databases, list_instance_configs, and list_backups will now return protos rather than the handwritten wrapper

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 2, 2020
@larkee larkee requested a review from busunkim96 October 6, 2020 02:31
@busunkim96
Copy link
Contributor

Thank you for working on this!

@larkee
Copy link
Contributor Author

larkee commented Oct 7, 2020

  • I noticed there doesn't seem to be a samples presubmit. I opened 335755839 to add those kokoro configs internally.

Thanks! I wasn't sure what was missing.

Handwritten files have been moved from google/cloud/spanner_v1 to google/cloud/spanner

I believe the other handwritten/generated mixes keep the generated surface in the versioned directory. What is the
motivation for the change?
https://github.com/googleapis/python-firestore/tree/master/google/cloud/firestore_v1
https://github.com/googleapis/python-pubsub/tree/v2.1.0/google/cloud/pubsub_v1

The motivation was two fold. It separates the handwritten surfaces which makes it clearer to contributes what is written and what is generated. It also prevents having to customize the generated google/cloud/spanner_v1/__init__.py. However given that this file shouldn't change too much, it's probably reasonable to exclude it from generation and manually update it when required.

I'm happy to revert these files back to their original folder if you think that would be best.

  • I'm not familiar with the Spanner client, are there particular files we should direct our attention to?

Most of the handwritten surfaces are used directly by users. The major ones are:

  • client.py
  • instance.py
  • database.py
  • backup.py
  • `batch.py
  • snapshot.py
  • transaction.py

However, the files that changed the most during migration are:

  • _helpers.py
  • snapshot.py
  • streamed.py
  • transaction.py

busunkim96
busunkim96 reviewed Oct 9, 2020
View reviewed changes
Copy link
Contributor

@busunkim96 busunkim96 left a comment

Choose a reason for hiding this comment

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

I would prefer the handwritten code live in the versioned directory for consistency with the other libraries. It looks like firestore excludes __init__.py in synth.py https://github.com/googleapis/python-firestore/blob/c3acd4a04745c93edb2f61bf9be6fa33f439f4b0/synth.py#L41

Fully generated libraries use the unversioned path google/cloud/{api}/ for the default alias (equivalent of google/cloud/spanner.py) https://github.com/googleapis/python-speech/blob/master/google/cloud/speech/__init__.py so there's also a small chance for confusion there.

google/cloud/spanner/snapshot.py Outdated Show resolved Hide resolved
@larkee larkee requested a review from busunkim96 October 26, 2020 04:53
busunkim96
busunkim96 approved these changes Oct 30, 2020
View reviewed changes
google/cloud/spanner.py Outdated Show resolved Hide resolved
google/cloud/spanner.py Show resolved Hide resolved
google/cloud/spanner_v1/streamed.py Outdated Show resolved Hide resolved
larkee added 22 commits November 3, 2020 16:06
@larkee
chore: remove old generated files
abb2281
@larkee
chore: remove type files
b660ddb
@larkee
refactor: move handwritten files to google/cloud/spanner
5114db3
@larkee
chore: update synth.py
cdf9795
@larkee
chore: regen library via microgenerator
ef6835b
@larkee
fix: update imports to use new files
848d576
@larkee
fix: update _helper.py
936efd3
@larkee
update backup.py
2969885
@larkee
fix: update batch.py
419ab72
@larkee
fix: update param_types.py
cc32144
@larkee
fix: update pool.py
9109bb0
@larkee
test: update test_keyset.py
47df3d7
@larkee
test: update test__opentelemetry_tracing.py
f1db0cc
@larkee
fix: update client.py
2ab6b3b
@larkee
test: update test_client.py
2f77e43
@larkee
fix: update database.py
eea1b8f
@larkee
test: update test_database.py
3a08a9a
@larkee
fix: update commmit call missing kwarg
e111612
@larkee
test: update test_session.py
241d71a
@larkee
fix: update streamed.py
8cdbe3d
@larkee
test: update TestStreamedResultSet tests in streamed.py
8335288
@larkee
fix: update instance.py
979a6d8
@larkee larkee force-pushed the generator-migration branch from d1c3dab to bf06add Compare November 3, 2020 06:30
@larkee larkee marked this pull request as ready for review November 5, 2020 07:16
@larkee larkee requested review from a team as code owners November 5, 2020 07:16
skuruppu
skuruppu approved these changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

@skuruppu skuruppu left a comment

Choose a reason for hiding this comment

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

I only reviewed UPGRADING.md so my LGTM is for that file.

UPGRADING.md Outdated Show resolved Hide resolved
@c24t c24t mentioned this pull request Nov 10, 2020
Closed
@larkee larkee merged commit bf4b278 into googleapis:master Nov 11, 2020
@release-please release-please bot mentioned this pull request Nov 11, 2020
Merged
gcf-merge-on-green bot pushed a commit that referenced this pull request Nov 11, 2020
@release-please
chore: release 2.0.0 (#163)
b61c3ea 
🤖 I have created a release \*beep\* \*boop\* 
---
## [2.0.0](https://www.github.com/googleapis/python-spanner/compare/v1.19.1...v2.0.0) (2020-11-11)


### ⚠ BREAKING CHANGES

* migrate to v2.0.0 (#147)

### Features

* DB-API driver + unit tests ([#160](https://www.github.com/googleapis/python-spanner/issues/160)) ([2493fa1](https://www.github.com/googleapis/python-spanner/commit/2493fa1725d2d613f6c064637a4e215ee66255e3))
* migrate to v2.0.0 ([#147](https://www.github.com/googleapis/python-spanner/issues/147)) ([bf4b278](https://www.github.com/googleapis/python-spanner/commit/bf4b27827494e3dc33b1e4333dfe147a36a486b3))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
@wyardley
Copy link

wyardley commented Dec 1, 2020

@larkee: In the end, the "breaking change" that shows up in the changelog was
migrate to v2.0.0 (#147) vs

list_instances, list_databases, list_instance_configs, and list_backups will now return protos rather than the handwritten wrapper

which would have been much more useful

@larkee
Copy link
Contributor Author

larkee commented Dec 2, 2020

@wyardley My apologies! I'll do my best to label changes better in future 👍 I'll update the changelog directly to show the more useful message

@larkee larkee mentioned this pull request Dec 2, 2020
Merged
@c24t c24t mentioned this pull request Dec 21, 2020
Closed
@rajatbhatta rajatbhatta mentioned this pull request Jan 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@busunkim96 busunkim96 busunkim96 approved these changes

@skuruppu skuruppu skuruppu approved these changes

@software-dov software-dov Awaiting requested review from software-dov

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@larkee @busunkim96 @wyardley @skuruppu