Skip to content

Adding one-time RPC to find unaliased / true dataset ID. - #528

Closed
dhermes wants to merge 1 commit into
googleapis:masterfrom
dhermes:non-fuzzy-app-id
Closed

Adding one-time RPC to find unaliased / true dataset ID.#528
dhermes wants to merge 1 commit into
googleapis:masterfrom
dhermes:non-fuzzy-app-id

Conversation

@dhermes

Copy link
Copy Markdown
Contributor

Also removing unnecessary functions that are no longer
needed since there is no need to muck with the dataset ID.

@tseaver This is really an exploratory PR, no need to review just yet.

@pcostell Can you check out the _find_true_dataset_id method in gcloud/datastore/__init__.py (feel free to ignore the rest).

Is this a dumb idea? Is there an actual API available to convert from the "human friendly" ID foo to the unaliased / prefixed s~foo?

@dhermesdhermes added api: datastore Issues related to the Datastore API. type: question Request for information or clarification. Not an issue. labels Jan 9, 2015
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same when pulling 78bd1a2 on dhermes:non-fuzzy-app-id into 0525530 on GoogleCloudPlatform:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same when pulling e50b270 on dhermes:non-fuzzy-app-id into 0525530 on GoogleCloudPlatform:master.

@pcostell

Copy link
Copy Markdown
Contributor

Pretty soon the datastore will normalize all types of keys, so you should never need to provide anything except the project id. Importantly, _find_true_dataset_id will only work with the current dataset which means it will break for users that store foreign keys.

In the near-future we don't want anyone to have to deal with the "s~", it should be purely an implementation detail of the Datastore. I think the client libraries should probably be written pretending the "s~" doesn't exist since it is cleaner that way (with the caveat that it is slightly broken atm).

@dhermes

Copy link
Copy Markdown
ContributorAuthor

Thanks for the quick followup. That is great news! It will certainly be much cleaner if we can ignore s~. All the deleted code in this PR will still be able to be deleted even without _find_true_dataset_id.

How do you mean normalize? Currently it seems some normalization is happening, but that means if I send no dataset ID in the request and foo as the dataset ID in the URI, then I get back s~foo on the keys in the response. This still causes some issues / confusion / surprise.

@pcostell

Copy link
Copy Markdown
Contributor

I believe the goal is that everywhere you will specify foo or an empty dataset and when you get the key back you will also only see foo in the dataset.

@eddavisson can you confirm?

@eddavisson

Copy link
Copy Markdown

That's correct. Note that if you want to do a one-time RPC in the mean time, you can do a Lookup request and check both the found and missing fields in the response for the resolved dataset id (basically what you're doing in this pull request but it's a read-only RPC).

@dhermes

Copy link
Copy Markdown
ContributorAuthor

I didn't think to use missing! Good call. With the __key__ projection that'd be cheap enough, even if somehow the user had stored Key('__Foo', 1).

UPDATE: DERP, no projection on lookup.

@silvolu

Copy link
Copy Markdown
Contributor

I think we should go forward with @eddavisson suggestion while waiting for the API updates to land.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same when pulling 3184184 on dhermes:non-fuzzy-app-id into 0525530 on GoogleCloudPlatform:master.

@dhermes

Copy link
Copy Markdown
ContributorAuthor

@pcostell RE: foreign keys. I'm not sure why that's an issue if the application is managing those keys.

I double checked and the backend does not change the foreign dataset ID after storing (seems it shouldn't, for privacy reasons). From 0525530:

>>>fromgcloudimportdatastore>>>fromgcloud.datastoreimport_implicit_environ>>>fromgcloud.datastore.entityimportEntity>>>fromgcloud.datastore.keyimportKey>>>>>>datastore.set_default_connection()
>>>dataset_id='foo'>>>datastore.set_default_dataset_id(dataset_id)
>>>>>>e=Entity(key=Key('Foo', 1))
>>>e['bar'] =Key('Baz', 1, dataset_id='foreignappid')
>>>e.save()
>>>>>>e_retrieved=datastore.get(e.key)
>>>e_retrieved['bar'].dataset_idu'foreignappid'>>>>>>entity_pb, =_implicit_environ.CONNECTION.lookup(
... dataset_id=dataset_id,
... key_pbs=[e.key.to_protobuf()],
... )
>>>entity_pb.property
[<gcloud.datastore.datastore_v1_pb2.Propertyobjectat0x7f4a5006d398>]
>>>printentity_pb.property[0].value.key_valuepartition_id {
dataset_id: "foreignappid"
}
path_element {
kind: "Baz"id: 1
}
>>>e.key.delete()

FWIW I tried this both with a foreign dataset that my (user, not service account) OAuth 2.0 token had access to and one that the token did not.

@pcostell

Copy link
Copy Markdown
Contributor

I believe we want to remove the notion of "s~" completely. So anywhere the user interacts with a dataset it should never have this extra prefix, including foreign keys.

Note that this is not implemented, so it isn't finalized and definitely won't work yet.

@dhermes
dhermesforce-pushed the non-fuzzy-app-id branch 2 times, most recently from ab4c103 to 0cf4333CompareJanuary 12, 2015 22:49
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same when pulling 0cf4333 on dhermes:non-fuzzy-app-id into dcb73d3 on GoogleCloudPlatform:master.

Comment threadgcloud/datastore/__init__.py Outdated

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 15, 2015
@dhermes

Copy link
Copy Markdown
ContributorAuthor

Had to rebase after #548 and #550, so did the renames in each commit through the rebase.

@coveralls

Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 31fd6fd on dhermes:non-fuzzy-app-id into * on GoogleCloudPlatform:master*.

This comment was marked as spam.

@tseaver

Copy link
Copy Markdown
Contributor

LGTM.

@dhermes

Copy link
Copy Markdown
ContributorAuthor

@tseaver We can't / shouldn't merge this until the discussion above is resolved.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same when pulling 5d631a6 on dhermes:non-fuzzy-app-id into 55285f9 on GoogleCloudPlatform:master.

Also removing unnecessary functions that are no longer
needed since there is no need to muck with the dataset ID.
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling 9b0f87a on dhermes:non-fuzzy-app-id into 9518db7 on GoogleCloudPlatform:master.

dhermes added a commit to dhermes/google-cloud-python that referenced this pull request Feb 19, 2015
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 added a commit to dhermes/google-cloud-python that referenced this pull request Feb 19, 2015
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

Copy link
Copy Markdown
ContributorAuthor

I'm closing this (since it is so out of date) and moving to discussion / tracking in #821.

Part of it was merged in #792 as well.

@dhermesdhermes closed this Apr 13, 2015
@dhermes
dhermes deleted the non-fuzzy-app-id branch April 13, 2015 21:40
parthea pushed a commit that referenced this pull request Aug 15, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/cb960373d12d20f8dc38beee2bf884d49627165e
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:2d816f26f728ac8b24248741e7d4c461c09764ef9f7be3684d557c9632e46dbd
vchudnov-g pushed a commit that referenced this pull request Sep 20, 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 pushed a commit that referenced this pull request Sep 22, 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
* docs: Fix formatting of request arg in docstring
chore: Update gapic-generator-python to v1.9.1
PiperOrigin-RevId: 518604533
Source-Link: googleapis/googleapis@8a085ae
Source-Link: googleapis/googleapis-gen@b2ab4b0
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjJhYjRiMGEwYWUyOTA3ZTgxMmMyMDkxOThhNzRlMDg5OGFmY2IwNCJ9
* 🦉 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 added a commit that referenced this pull request Oct 21, 2023
* feat: Add support for python 3.11
chore: Update gapic-generator-python to v1.8.0
PiperOrigin-RevId: 500768693
Source-Link: googleapis/googleapis@190b612
Source-Link: googleapis/googleapis-gen@7bf29a4
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiN2JmMjlhNDE0YjllY2FjMzE3MGYwYjY1YmRjMmE5NTcwNWMwZWYxYSJ9
* 🦉 Updates from OwlBot post-processor
See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md
* update setup.py
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this pull request Oct 21, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
@release-pleaserelease-pleaseBot mentioned this pull request Oct 30, 2023
parthea added a commit that referenced this pull request Aug 21, 2025
* chore(deps): update all dependencies
* revert ipython
* add dependency ipywidgets
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this pull request Sep 16, 2025
* chore(deps): update all dependencies
* revert ipython
* add dependency ipywidgets
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this pull request Nov 22, 2025
Source-Link: googleapis/synthtool@82f5cb2
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:5d8da01438ece4021d135433f2cf3227aa39ef0eaccc941d62aa35e6902832ae
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea added a commit that referenced this pull request Nov 24, 2025
parthea pushed a commit that referenced this pull request Nov 24, 2025
…528)
The emulator is different enough from the real Datastore to require some
special handling.
Fixes#525
parthea pushed a commit that referenced this pull request Nov 26, 2025
#528)
* Narrow acceptable RSA versions to maintain Python 2 compat
* Update setup.py
Co-authored-by: Kamil Breguła <mik-laj@users.noreply.github.com>
* Use a more specific pin to support new versions that may support python 2.
* Update setup.py
Co-authored-by: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>
* how many commits to get the format correct?
Co-authored-by: Kamil Breguła <mik-laj@users.noreply.github.com>
Co-authored-by: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Nov 26, 2025
🤖 I have created a release \*beep\* \*boop\* ---
### [1.17.1](https://www.github.com/googleapis/google-auth-library-python/compare/v1.17.0...v1.17.1) (2020-06-11)
### Bug Fixes
* narrow acceptable RSA versions to maintain Python 2 compatability ([#528](https://www.github.com/googleapis/google-auth-library-python/issues/528)) ([9434868](https://www.github.com/googleapis/google-auth-library-python/commit/9434868a6789464549af1d4562f62d8a899b6809))
---
This PR was generated with [Release Please](https://github.com/googleapis/release-please).
parthea pushed a commit that referenced this pull request Nov 26, 2025
Related to #528. RSA seems to have released another version without `python_requires` being enforced. This will guard against that for our package.
parthea pushed a commit that referenced this pull request Mar 2, 2026
parthea pushed a commit that referenced this pull request Mar 9, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: datastoreIssues related to the Datastore API.cla: yesThis human has signed the Contributor License Agreement.type: questionRequest for information or clarification. Not an issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@dhermes@coveralls@pcostell@eddavisson@silvolu@tseaver@googlebot