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

fix: location prefix support #327

Closed
wants to merge 4 commits into from
Closed

Conversation

chmoder
Copy link
Contributor

@chmoder chmoder commented Feb 6, 2020

This PR has two ideas implemented, only one may need to be pulled.

Commit 1 - optionally include location prefix when creating ndb.Key instances
Commit 2 - use a secondary method legacy_urlsafe to include location prefix in urlsafe key.

Thanks for the support and updates. We look forward to communicating the best way to include location prefix.

NOTE: I noticed that adding location prefix caused tests that expect the location prefix to be stripped away to fail. We can help resolve these tests by anticipating the prefix to be kept.

Fixes: #264

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 6, 2020
@chmoder chmoder changed the title Location Prefix Support fix Add Location Prefix Support Feb 6, 2020
@chmoder chmoder changed the title fix Add Location Prefix Support Add Location Prefix Support Feb 6, 2020
@chmoder chmoder changed the title Add Location Prefix Support fix: add location prefix support Feb 6, 2020
@chmoder chmoder changed the title fix: add location prefix support fix: location prefix support Feb 6, 2020
@chrisrossi
Copy link
Contributor

When I looked at #264, the reason I marked it "blocked" was because I couldn't find a way to get the prefix. I thought maybe you had found something I hadn't, but when I look at your solution, it's something I had already tried. I just don't get a prefix when I do this:

Python 3.7.5 (default, Nov 20 2019, 09:21:52) 
[GCC 9.2.1 20191008] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from google.cloud import ndb
>>> client = ndb.Client()
>>> client.project
'ndb-system-tests-chrisrossi'
>>> with client.context() as context:
...     print(context.client.project)
... 
ndb-system-tests-chrisrossi
>>> 

The problem I found is that the Datastore GUI was giving us ursafe keys with the prefix, but we had no way to get the prefix on the client side. I still don't see a way forward on that.

Am I missing something?

@chmoder
Copy link
Contributor Author

chmoder commented Feb 10, 2020

Thanks Chris,

We are actually trying to preserve original ndb functionality - so when we serialize a key to urlsafe we would like to have the location_prefix included. I've also seen this called region_code as seen on GAE documentation

I tried not to change the interfaces as much as possible. However, there are certainly other ways to do this. For example, add location_prefix as a KWARG to ndb.Client().

Commit 1 example:

from google.cloud import ndb
client = ndb.Client(project='s~project-id')
project = ndb_client.client.project

Commit 2 example:

from google.cloud import ndb
client = ndb.Client(project='project-id')
key = ndb.Key('SomeKind', 1234)
urlsafe_key = key.legacy_urlsafe(location_prefix='s~')

@chrisrossi
Copy link
Contributor

Ok, so you're just presuming that the programmer already knows the prefix. That can probably work as a compromise. Let's see if @andrewsg gets anything out of the Datastore team about this.

@andrewsg
Copy link
Contributor

andrewsg commented Mar 2, 2020

Integrated in #348

@andrewsg andrewsg closed this Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

urlsafe ndb key is different than datastore admin
4 participants