Skip to content

Adding lazy loading support for datastore connection. #668

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
Merged
dhermes merged 4 commits into from
Feb 25, 2015
Merged

Adding lazy loading support for datastore connection. #668

dhermes merged 4 commits into from
Feb 25, 2015

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Feb 19, 2015

NOTE: Has #667 as diffbase.

Happens in 4 phases:

  • Remove datastore.helpers import in connection. (this would have caused a cycle if Connection was used in _implicit_environ)
  • Moving get_connection() from datastore.__init__ to _implicit_environ.
  • Moving set_default_connection() from datastore.__init__ to _implicit_environ.
  • Adding lazy loading support for the connection property

Note that I had to copy _prepare_key_for_request from helpers into connection to avoid cyclic imports (#528 would be nice, since we could just remove _prepare_key_for_request all together)

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 19, 2015
@dhermes dhermes added the api: datastore Issues related to the Datastore API. label Feb 19, 2015
@coveralls
Copy link

coveralls commented Feb 19, 2015

Coverage Status

Coverage remained the same at 100.0% when pulling fa04127 on dhermes:lazy-loading-attempt-5 into 72b6359 on GoogleCloudPlatform:master.

@dhermes dhermes mentioned this pull request Feb 19, 2015
Closed
tseaver
tseaver reviewed Feb 19, 2015
View reviewed changes
gcloud/datastore/_implicit_environ.py Outdated

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Contributor

tseaver commented Feb 19, 2015

The last commit LGTM, except for my note about the _method name.

@dhermes dhermes mentioned this pull request Feb 19, 2015
Merged
@dhermes
Remove datastore.helpers import in connection.
77f19bd 
Done via:
- Moving `_add_keys_to_request` definition to connection module
- Removing only other use (in `batch`, was not a list of keys)
- Copying `_prepare_key_for_request` from helpers (googleapis#528 would be
  nice, since we could just remove `_prepare_key_for_request`
  all together)
@dhermes
Moving get_connection() from datastore.__init__ to _implicit_environ.
7be6a1d
@dhermes
Moving set_default_connection() to _implicit_environ.
d0fb459 
Moved from datastore.__init__.
@dhermes
Adding lazy loading support for datastore connection.
7b0fc6e
@dhermes dhermes force-pushed the lazy-loading-attempt-5 branch from fa04127 to 7b0fc6e Compare February 19, 2015 21:09
@dhermes
Copy link
Contributor Author

dhermes commented Feb 19, 2015

@tseaver PTAL I rebased on top of #667.

Your _method comment was resolved in #667. You gave the LGTM for the final commit, but this PR actually involved 3 commits to move code before (on of which involves a duplicated method and pragma NO COVER worth discussing)

Also the commit you LGTM'd changed slightly since I changed #667 to use a decorator for lazy properties

@coveralls
Copy link

coveralls commented Feb 19, 2015

Coverage Status

Coverage remained the same at 100.0% when pulling 7b0fc6e on dhermes:lazy-loading-attempt-5 into 56f60bb on GoogleCloudPlatform:master.

tseaver
tseaver reviewed Feb 19, 2015
View reviewed changes
gcloud/datastore/batch.py

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Contributor

tseaver commented Feb 19, 2015

I've got to run, won't be able to finish review tonight. :(

@dhermes
Copy link
Contributor Author

dhermes commented Feb 19, 2015

No worries

This was referenced Feb 20, 2015
Merged
Merged
@dhermes
Copy link
Contributor Author

dhermes commented Feb 24, 2015

@tseaver After this is in I'd like to cut 0.4.2. WDYT?

@tseaver
Copy link
Contributor

tseaver commented Feb 25, 2015

OK, LGTM. 0.4.2 would be fine, as well.

dhermes added a commit that referenced this pull request Feb 25, 2015
@dhermes
Merge pull request #668 from dhermes/lazy-loading-attempt-5
0e41dc3 
Adding lazy loading support for datastore connection.
@dhermes dhermes merged commit 0e41dc3 into googleapis:master Feb 25, 2015
@dhermes dhermes deleted the lazy-loading-attempt-5 branch February 25, 2015 02:13
@dhermes dhermes mentioned this pull request Feb 25, 2015
Merged
parthea pushed a commit that referenced this pull request Jul 6, 2023
@jmdobry @busunkim96
Add Cloud Client NL API samples. [(#668)](GoogleCloudPlatform/python-…
464ac55 
…docs-samples#668)
vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
@gcf-owl-bot
feat: added speech endpointing setting (#668)
2da7a77 
* feat: added speech endpointing setting
feat: added Knowledge Search API

PiperOrigin-RevId: 560215389

Source-Link: googleapis/googleapis@b1666e6

Source-Link: googleapis/googleapis-gen@07e82b4
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMDdlODJiNGQ0OTE3NDk0ZDg1MDM5NzIyZjY3M2Q0MDljNGM3MzA1ZiJ9

* 🦉 Updates from OwlBot post-processor

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

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Nov 22, 2025
@gcf-owl-bot
chore: use gapic-generator-python 1.3.1 (#668)
c0c9260 
- [ ] Regenerate this pull request now.

PiperOrigin-RevId: 472772457

Source-Link: googleapis/googleapis@855b74d

Source-Link: googleapis/googleapis-gen@b64b1e7
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjY0YjFlN2RhM2UxMzhmMTVjYTM2MTU1MmVmMDU0NWU1NDg5MWI0ZiJ9
parthea added a commit that referenced this pull request Nov 24, 2025
@parthea @daniel-sanche @gcf-owl-bot
chore: move retry async check to wrap time (#668)
0ff5b08 
* chore: move retry async check to wrap time

* 🦉 Updates from OwlBot post-processor

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

* added no cover mark

---------

Co-authored-by: Daniel Sanche <[email protected]>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Daniel Sanche <[email protected]>
parthea added a commit that referenced this pull request Nov 24, 2025
@parthea
chore: Revert "chore: move retry async check to wrap time (#668)" (#743)
ab1d30f 
This reverts commit 3c698679e544df571ed5e181db29b2d55eafbe2b.
parthea pushed a commit that referenced this pull request Nov 24, 2025
@daniel-sanche
chore: fix environment tests by changing uuid generation (#668)
3525785
parthea pushed a commit that referenced this pull request Nov 25, 2025
@daniel-sanche
chore: fix environment tests by changing uuid generation (#668)
57ddaaa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

api: datastore Issues related to the Datastore API. 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

@dhermes @coveralls @tseaver @googlebot