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

Address seventh part of 451: Make dataset_id required on Key. #463

Merged
merged 2 commits into from
Dec 31, 2014
Merged

Address seventh part of 451: Make dataset_id required on Key. #463

merged 2 commits into from
Dec 31, 2014

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Dec 23, 2014

Addresses seventh part of #451.

Also requires removing the dataset_id from generated protobufs
due to googleapis/google-cloud-datastore#59. This
occurs also in __key__ filters and ancestor queries.

NOTE: This has #462 as a diffbase.

@dhermes dhermes mentioned this pull request Dec 23, 2014
Closed
10 tasks
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0321c41 on dhermes:fix-451-part7 into b343acf on GoogleCloudPlatform:master.

@tseaver
Copy link
Contributor

tseaver commented Dec 30, 2014

0321c41 comments inline on the commit.

@dhermes dhermes changed the title Address sixth part of 451: Make dataset_id required on Key. Address seventh part of 451: Make dataset_id required on Key. Dec 30, 2014
@dhermes
Copy link
Contributor Author

dhermes commented Dec 30, 2014

RE: "Mutating key_pb in helper._prepare_key_for_request". It's fine with me, I just didn't want to be presumptuous about callers expectations. If users ever use the public methods on Connection directly, this would cause side effects they may not expect. I also considered adding a flag to Key.to_protobuf() that would allow dropping the dataset_id at creation time.

RE: "Requiring dataset_id in Key constructor". You are right it isn't truly required, but Key instantiation will fail if the implicit one is not set. Should I just add a note about this higher up the docstring?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0e57202 on dhermes:fix-451-part7 into 8cb41f6 on GoogleCloudPlatform:master.

@dhermes dhermes mentioned this pull request Dec 30, 2014
Merged
@tseaver
Copy link
Contributor

tseaver commented Dec 31, 2014

I'm OK with leaving _prepare_key_for_request as is.

Maybe the :param dataset_id docstring should say, "Required, unless the implicit dataset ID has been set."

@dhermes dhermes force-pushed the fix-451-part7 branch 2 times, most recently from e1ed6c5 to cc93433 Compare December 31, 2014 22:15
tseaver
tseaver reviewed Dec 31, 2014
View reviewed changes
gcloud/datastore/test_key.py
pb = key.to_protobuf()
pb.partition_id.namespace = 'NAMESPACE'
self.assertRaises(ValueError, key.compare_to_proto, pb)
>>>>>>> 0e57202... Make dataset_id required on Key.

This comment was marked as spam.

Sign in to view

@dhermes
Make dataset_id required on Key.
48d2015 
Addresses seventh part of googleapis#451.

Also requires removing the dataset_id from generated protobufs
due to googleapis/google-cloud-datastore#59. This
occurs also in __key__ filters and ancestor queries.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 48d2015 on dhermes:fix-451-part7 into e86ba09 on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 31, 2014

@tseaver PTAL (removing compare_to_proto turned out to make git unhappy on the rebase)

tseaver
tseaver reviewed Dec 31, 2014
View reviewed changes
gcloud/datastore/key.py
returned from the datastore backend. The application
**must** treat any value set by the back-end as opaque.
:param dataset_id: The dataset ID associated with the key. This is
required. Can only be passed as a keyword argument.

This comment was marked as spam.

Sign in to view

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f697464 on dhermes:fix-451-part7 into e86ba09 on GoogleCloudPlatform:master.

tseaver added a commit that referenced this pull request Dec 31, 2014
@tseaver
Merge pull request #463 from dhermes/fix-451-part7
8b04561 
Address seventh part of 451: Make dataset_id required on Key.
@tseaver tseaver merged commit 8b04561 into googleapis:master Dec 31, 2014
@dhermes dhermes deleted the fix-451-part7 branch December 31, 2014 22:38
@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
@gcf-owl-bot
feat: Added Training and Evaluation functions, request, responses and…
ca22baf 
… metadata to document_processor_service.proto (#463)

* feat: Added Training and Evaluation functions, request, responses and metadata to document_processor_service.proto
feat: Added evaluation.proto
feat: Added latest_evaluation to processor.proto
chore: removed deprecated flag from REPLACE in OperationType in document.proto

PiperOrigin-RevId: 511230520

Source-Link: googleapis/googleapis@c53bf8d

Source-Link: googleapis/googleapis-gen@5693cbc
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNTY5M2NiYzk4YjY0MTIwODVmZjZkM2FiYTcxOWI2OGUzMDdjNTM1NyJ9

* 🦉 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
@gcf-owl-bot
build(deps): bump cryptography from 38.0.3 to 39.0.1 in /synthtool/gc…
62b95f5 
…p/templates/python_library/.kokoro (#463)

Source-Link: https://github.com/googleapis/synthtool/commit/bb171351c3946d3c3c32e60f5f18cee8c464ec51
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f62c53736eccb0c4934a3ea9316e0d57696bb49c1a7c86c726e9bb8a2f87dadf
parthea pushed a commit that referenced this pull request Aug 15, 2023
@release-please
chore(main): release 1.13.4 (#463)
b282cf6 
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
@gcf-owl-bot @parthea
ci(python): run lint / unit tests / docs as GH actions (#463)
00ef1f8 
Source-Link: googleapis/synthtool@57be0cd
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:ed1f9983d5a935a89fe8085e8bb97d94e41015252c5b6c9771257cf8624367e6

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 Sep 22, 2023
@renovate-bot @parthea
chore(deps): update dependency google-cloud-bigquery to v3.11.0 (#463)
b8f0ada 
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this pull request Sep 22, 2023
@renovate-bot @parthea
chore(deps): update all dependencies (#463)
37d3fa4 
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this pull request Oct 21, 2023
@release-please
chore(main): release 3.11.0 (#463)
13fc8aa 
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Oct 21, 2023
@gcf-owl-bot
chore(python): allow client documentation to be customized in README (#…
a7514a1 
…463)

Source-Link: googleapis/synthtool@95d9289
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:c8878270182edaab99f2927969d4f700c3af265accd472c3425deedff2b7fd93

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Oct 21, 2023
@renovate-bot
chore(deps): update dependency google-cloud-storage to v2.6.0 (#463)
58f8133
parthea added a commit that referenced this pull request Oct 21, 2023
@renovate-bot @gcf-owl-bot @parthea
chore(deps): update all dependencies (#463)
e4b3cd8 
* 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>
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dhermes @coveralls @tseaver