-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Removing hacks that avoid using project ID in key protos. #1466
Conversation
LGTM pending @pcostell, unless you want to remove |
Correct, the project_id will never contain cluster info. From our release notes (current hidden behind a whitelist):
|
Thanks @pcostell! I confirmed by trying to send I noticed some extra bytes in the error
It seems the error is type |
This was because returned dataset IDs in v1beta2 turned foo into s~foo and sometimes this caused mismatches.
8434764
to
090b187
Compare
@jonparrott PTAL. I added another commit to remove the prefix checking. I love deleting code. |
LGTM. I love watching you delete code. (There is a surprisingly large amount of code for handling Datastore) |
Removing hacks that avoid using project ID in key protos.
The error is documented here. Are you sure it's a DebugInfo? It should be a google.rpc.Status message. |
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." |
This was because returned dataset IDs in
v1beta2
turnedfoo
intos~foo
and sometimes this caused mismatches.@pcostell Do we still need
foo
ands~foo
to be considered "equal" inv1beta3
?I held off on removing the custom
_projects_equal
, which is used inKey.__eq__
,Batch.put
,Batch.delete
andClient.get_multi
. However, by making_projects_equal
just checkproject1 == project2
the system tests pass just fine. (This would fix #821)