FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Using protobuf CommitRequest in datastore Connection.commit. by dhermes · Pull Request #1341 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

Using protobuf CommitRequest in datastore Connection.commit. - #1341

Merged
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:datastore-commit-use-commit-req
Jan 7, 2016
ConversationCommits (1)ChecksFiles changed
Merged

Using protobuf CommitRequest in datastore Connection.commit.#1341
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:datastore-commit-use-commit-req

Conversation

dhermes commented Jan 3, 2016

Copy link
Copy Markdown
Contributor

This is towards #1288 in preparation for the upgrade to v1beta3. In particular, a single Mutation protobuf instance in v1beta3 is not sufficient to contain all changes to be committed, so we use the container that is up one level in the hierarchy.

dhermes added the api: datastore Issues related to the Datastore API. label Jan 3, 2016
googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 3, 2016
dhermes mentioned this pull request Jan 3, 2016
Closed
49 tasks
tseaver reviewed Jan 7, 2016
View reviewed changes
Comment thread gcloud/datastore/connection.py

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

tseaver commented Jan 7, 2016

Copy link
Copy Markdown
Contributor

It makes me queasy to think about reusing a batch or a transaction. If somebody wanted to examine the _commit_request after commit, that would be fine, but I don't see a case for resetting it.

Using protobuf CommitRequest in datastore Connection.commit.
7f1b786 
This is towards googleapis#1288 in preparation for the upgrade to `v1beta3`.
In particular, a single `Mutation` protobuf instance in `v1beta3` is not
sufficient to contain all changes to be committed, so we use the
container that is up one level in the hierarchy.
dhermes force-pushed the datastore-commit-use-commit-req branch from a1194f5 to 7f1b786 Compare January 7, 2016 23:01

dhermes commented Jan 7, 2016

Copy link
Copy Markdown
Contributor Author

OK. Should I file an issue to move the tombstone behavior into Batch? Also, would it be better to use the passed in commit_request after moving that behavior or should I do it in this PR?

PS I just rebased, had a merge issue.

tseaver commented Jan 7, 2016

Copy link
Copy Markdown
Contributor

I think we could just open an issue and go on.

dhermes commented Jan 7, 2016

Copy link
Copy Markdown
Contributor Author

LGTY to merge?

dhermes mentioned this pull request Jan 7, 2016
Closed

dhermes commented Jan 7, 2016

Copy link
Copy Markdown
Contributor Author

Filed #1366

tseaver commented Jan 7, 2016

Copy link
Copy Markdown
Contributor

LGTM

dhermes added a commit that referenced this pull request Jan 7, 2016
Merge pull request #1341 from dhermes/datastore-commit-use-commit-req
972e875 
Using protobuf CommitRequest in datastore Connection.commit.
dhermes merged commit 972e875 into googleapis:master Jan 7, 2016
dhermes deleted the datastore-commit-use-commit-req branch January 7, 2016 23:15
dhermes mentioned this pull request Jan 25, 2016
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL