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

Implicit dataset from environ #444

Merged
merged 2 commits into from
Dec 30, 2014

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Dec 19, 2014

This has #436 as a diff-base, so feel free to hold off review until it is merged.
This adds helper methods for implicit datastore environ.

By setting two environment variables

  • GCLOUD_DATASET_ID
  • GOOGLE_APPLICATION_CREDENTIALS

the user can call convenience methods in gcloud.datastore directly
without worrying about auth or the name of the dataset. The
goal is that in places like App Engine and Compute Engine, the
dataset ID can be implied without any user intervention.

Partially addresses #337.

NOTE: This still needs to be documented, but it's unclear where is appropriate. We also need to have documentation for auth (outside of CONTRIBUTING.md).

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2f227ac on dhermes:implicit-dataset-from-environ into 89ded75 on GoogleCloudPlatform:master.

@dhermes dhermes force-pushed the implicit-dataset-from-environ branch from 2f227ac to 13c727b Compare December 19, 2014 01:09
@dhermes
Copy link
Contributor Author

dhermes commented Dec 19, 2014

Also note this can't be merged until the comment in Entity.__init__ is resolved:

    def __init__(self, dataset=None, kind=None, exclude_from_indexes=()):
        super(Entity, self).__init__()
        # DJH: Need to decide if this inherits from object/dict. Notice that
        #      `Entity` objects are False-y even if they have protected
        #      variables set.

This is the question put forth in #396. The short term solution is to inherit from both dict and the helper that Query and Transaction inherit from:

class Entity(dict, _implicit_environ._DatastoreBase):
    ...

Though branching MRO is something to avoid if we can.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 13c727b on dhermes:implicit-dataset-from-environ into 21e5f45 on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 19, 2014

@silvolu This was one of the "convenience" things I mentioned before.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f0a7825 on dhermes:implicit-dataset-from-environ into * on GoogleCloudPlatform:master*.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 19, 2014

@tseaver PTAL

@dhermes dhermes force-pushed the implicit-dataset-from-environ branch from f0a7825 to 153f306 Compare December 20, 2014 01:44
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 153f306 on dhermes:implicit-dataset-from-environ into b343acf on GoogleCloudPlatform:master.

@dhermes dhermes mentioned this pull request Dec 21, 2014
@dhermes dhermes force-pushed the implicit-dataset-from-environ branch from 153f306 to c5457aa Compare December 22, 2014 20:34
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c5457aa on dhermes:implicit-dataset-from-environ into b343acf on GoogleCloudPlatform:master.

By setting two environment variables

- GCLOUD_DATASET_ID
- GOOGLE_APPLICATION_CREDENTIALS

the user can call convenience methods in `gcloud.datastore` directly
without worrying about auth or the name of the dataset. The
goal is that in places like App Engine and Compute Engine, the
dataset ID can be implied without **any** user intervention.

Partially addresses googleapis#337.

NOTE: This still needs to be documented, but it's unclear
      where is appropriate. We also need to have documentation
      for auth (outside of CONTRIBUTING.md).
@dhermes dhermes force-pushed the implicit-dataset-from-environ branch from c5457aa to d85d200 Compare December 22, 2014 21:05
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d85d200 on dhermes:implicit-dataset-from-environ into b343acf on GoogleCloudPlatform:master.

:raises: :class:`EnvironmentError` if DATASET is not set.
"""
if _implicit_environ.DATASET is None:
raise EnvironmentError('Dataset could not be implied.')

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Dec 23, 2014

I'm pretty opposed to "implicit majyk" that triggers at import time, with no way for the application to control or opt out of. I'd much rather have the application do an explicit configuration call, either:

from gcloud.datastore import guess_default_dataset
guess_default_dataset(os.environ)

or have a way to register the dataset ID explicity (e.g., from a value in a config file)::

from gcloud.datastore import set_default_dataset
set_default_dataset(config.settings['gcloud.dataset_id'])

Yes, it requires a tiny bit more typing on a host where the environment contains the "right stuff", but it makes the process explicit, and a lot easier to explain / understand.

Incorporates feedback from @tseaver. Also
- Fixing bad tests that accidentally leave side-effects.
- Making the method to set the default dataset public.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7cd77f2 on dhermes:implicit-dataset-from-environ into b343acf on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 23, 2014

@tseaver I agree explicit is better, PTAL at latest changes. Thanks for the insight, I could have put more effort into the design of the interface.

RE: The MRO comment in Entity.__init__ it's moot since we are dropping dataset from Entity long term.


Also note RE: defaults set by free functions in datastore.__init__: when we remove Dataset we will need to also provide a method which sets a default connection and then have things like

q = datastore.query('Kind')

which will use both the default dataset ID and the default connection (bound to the query).

It's more likely that it'll only be necessary to use the default Connection in

q.fetch()

and just provide a

q.fetch(connection=connection)

to override.

@tseaver
Copy link
Contributor

tseaver commented Dec 30, 2014

LGTM, with the note on raising an exception in 'set_default_dataset' if no value is passed and we can't figure one out from the environment.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 30, 2014

Going to merge this, then rebase in #456 and merge that.

I'm not 100% sure about raising when set_default_dataset fails. You're probably right, but that's an easy change and GCE and GAE support still need to be added to the method so there is a lot to do.

dhermes added a commit that referenced this pull request Dec 30, 2014
@dhermes dhermes merged commit 563a19f into googleapis:master Dec 30, 2014
@dhermes dhermes deleted the implicit-dataset-from-environ branch December 30, 2014 16:53
@dhermes dhermes added the api: datastore Issues related to the Datastore API. label Dec 31, 2015
parthea pushed a commit that referenced this pull request Jun 4, 2023
* feat: exposed GetProcessorType to v1beta3

PiperOrigin-RevId: 502616281

Source-Link: googleapis/googleapis@04e6db9

Source-Link: googleapis/googleapis-gen@f4bc77f
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZjRiYzc3ZjkwNWU2ODA1N2RiMDliNzgzOTdhZmJkZGI5Yjg2YzgwZSJ9

* 🦉 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 Jun 4, 2023
* chore: Update gapic-generator-python to v1.9.0

PiperOrigin-RevId: 517425588

Source-Link: googleapis/googleapis@33c93eb

Source-Link: googleapis/googleapis-gen@d5f5978
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZDVmNTk3ODlkMTlmYzQzMjcwZmYyMTI0OTY3ZDRlYzg5OTJiOGU4ZiJ9

* 🦉 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>
vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
Source-Link: googleapis/synthtool@0941ef3
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:2f90537dd7df70f6b663cd654b1fa5dee483cf6a4edcfd46072b2775be8a23ec

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Sep 22, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/5f2a6089f73abf06238fe4310f6a14d6f6d1eed3
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:8555f0e37e6261408f792bfd6635102d2da5ad73f8f09bcb24f25e6afb5fac97
parthea added a commit that referenced this pull request Sep 22, 2023
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this pull request Sep 22, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/0ddbff8012e47cde4462fe3f9feab01fbc4cdfd6
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:bced5ca77c4dda0fd2f5d845d4035fc3c5d3d6b81f245246a36aee114970082b
parthea added a commit that referenced this pull request Oct 21, 2023
* chore: Update gapic-generator-python to v1.11.7

PiperOrigin-RevId: 573230664

Source-Link: googleapis/googleapis@93beed3

Source-Link: googleapis/googleapis-gen@f4a4eda
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZjRhNGVkYWE4MDU3NjM5ZmNmNmFkZjkxNzk4NzIyODBkMWE4ZjY1MSJ9

* 🦉 Updates from OwlBot post-processor

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

* chore: Update gapic-generator-python to v1.11.8

PiperOrigin-RevId: 574178735

Source-Link: googleapis/googleapis@7307199

Source-Link: googleapis/googleapis-gen@ce3af21
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiY2UzYWYyMWI3YzU1OWE4N2MyYmVmYzA3NmJlMGUzYWVkYTNhMjZmMCJ9

* 🦉 Updates from OwlBot post-processor

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

* chore: Update gapic-generator-python to v1.11.9

PiperOrigin-RevId: 574520922

Source-Link: googleapis/googleapis@5183984

Source-Link: googleapis/googleapis-gen@a59af19
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYTU5YWYxOWQ0YWM2NTA5ZmFlZGYxY2MzOTAyOTE0MWI2YTViODk2OCJ9

* 🦉 Updates from OwlBot post-processor

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

* update post processor image; remove unused files

* update docs/index.rst

* 🦉 Updates from OwlBot post-processor

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

* 🦉 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>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this pull request Oct 21, 2023
* chore: test minimum dependencies in python 3.7

* update constraints
parthea added a commit that referenced this pull request Oct 21, 2023
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this pull request Oct 21, 2023
* chore(deps): update all dependencies

* 🦉 Updates from OwlBot post-processor

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

* revert

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this pull request Oct 21, 2023
* fix(deps): allow protobuf 3.19.5

* explicitly exclude protobuf 4.21.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants