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

Removing hacks that avoid using project ID in key protos. #1466

Merged

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Feb 13, 2016

This was because returned dataset IDs in v1beta2 turned foo into s~foo and sometimes this caused mismatches.

@pcostell Do we still need foo and s~foo to be considered "equal" in v1beta3?

I held off on removing the custom _projects_equal, which is used in Key.__eq__, Batch.put, Batch.delete and Client.get_multi. However, by making _projects_equal just check project1 == project2 the system tests pass just fine. (This would fix #821)

@dhermes dhermes added the api: datastore Issues related to the Datastore API. label Feb 13, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 13, 2016
@dhermes dhermes mentioned this pull request Feb 13, 2016
49 tasks
@theacodes
Copy link
Contributor

LGTM pending @pcostell, unless you want to remove _projects_equal in another PR.

@pcostell
Copy link
Contributor

Correct, the project_id will never contain cluster info. From our release notes (current hidden behind a whitelist):

Field dataset_id is now project_id. project_id never includes the app/project ID "prefix" (e.g. "s~"). This fixes googleapis/google-cloud-datastore#59.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 13, 2016

Thanks @pcostell! I confirmed by trying to send s~ and just watching it fail (which is good).

I noticed some extra bytes in the error

'\x08\x05\x121The project s~omega-moonlight-697 does not exist.'

It seems the error is type DebugInfo (available in google/rpc/error_details). Is there some documentation somewhere which describes the errors? AFAIK the error message bodies in v1beta2 are all plain text.

This was because returned dataset IDs in v1beta2 turned
foo into s~foo and sometimes this caused mismatches.
@dhermes
Copy link
Contributor Author

dhermes commented Feb 13, 2016

@jonparrott PTAL. I added another commit to remove the prefix checking. I love deleting code.

@theacodes
Copy link
Contributor

LGTM. I love watching you delete code.

(There is a surprisingly large amount of code for handling Datastore)

dhermes added a commit that referenced this pull request Feb 13, 2016
Removing hacks that avoid using project ID in key protos.
@dhermes dhermes merged commit 4f6930b into googleapis:datastore-v1beta3 Feb 13, 2016
@dhermes dhermes deleted the remove-dataset-prefixing branch February 13, 2016 23:35
@pcostell
Copy link
Contributor

The error is documented here. Are you sure it's a DebugInfo? It should be a google.rpc.Status message.

@dhermes
Copy link
Contributor Author

dhermes commented Feb 14, 2016

Thanks @pcostell. Verified

>>> from google.rpc import status_pb2
>>> status_pb2.Status.FromString('\x08\x05\x121The project s~omega-moonlight-697 does not exist.')
code: 5
message: "The project s~omega-moonlight-697 does not exist."

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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants