Skip to content

support datastore transaction options#2613

Merged
neozwu merged 8 commits intogoogleapis:masterfrom
neozwu:transactional-options
Jan 31, 2018
Merged

support datastore transaction options#2613
neozwu merged 8 commits intogoogleapis:masterfrom
neozwu:transactional-options

Conversation

@neozwu
Copy link
Contributor

@neozwu neozwu commented Nov 10, 2017

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 10, 2017
@garrettjonesgoogle
Copy link
Member

garrettjonesgoogle commented Nov 14, 2017

Removing myself as reviewer since this is WIP and @vam-google is secondary on Datastore.

@garrettjonesgoogle garrettjonesgoogle removed their request for review November 14, 2017 02:43
@neozwu neozwu changed the title WIP: implementing datastore transaction options support datastore transaction options Nov 14, 2017
@neozwu neozwu requested a review from vam-google November 14, 2017 19:49
@neozwu
Copy link
Contributor Author

neozwu commented Nov 20, 2017

Kindly ping

@neozwu neozwu added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 20, 2017
vam-google
vam-google reviewed Nov 20, 2017
View reviewed changes
google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java Outdated
}
}

void setPrevTransactionId(Transaction transaction) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java Outdated
if (transaction.isActive()) {
transaction.rollback();
}
if (options.getModeCase().equals(TransactionOptions.ModeCase.READ_WRITE)) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java Outdated
}

@Override
public <T> T runInTransaction(final TransactionCallable<T> callable, TransactionOptions options) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-datastore/src/main/java/com/google/cloud/datastore/TransactionImpl.java Outdated

@Override
public ByteString getTransactionId() {
return transaction;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java Outdated
options =
options
.toBuilder()
.setReadWrite(

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java Outdated
}

void setPrevTransactionId(Transaction transaction) {
options =

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-datastore/src/main/java/com/google/cloud/datastore/Datastore.java Outdated
*
* @param <T> the type of the return value
*/
interface ReadWriteTransactionCallable<T> {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@neozwu
Copy link
Contributor Author

neozwu commented Nov 28, 2017

PTAL @vam-google

vam-google
vam-google approved these changes Dec 8, 2017
View reviewed changes
Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with few minor comments.

google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreHelperTest.java
assertEquals(entity2, values.get(1));
verify(transaction);
}

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-datastore/pom.xml Outdated
<groupId>com.google.cloud.datastore</groupId>
<artifactId>datastore-v1-proto-client</artifactId>
<version>1.4.1</version>
<version>1.5.1</version>

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

@garrettjonesgoogle
Copy link
Member

garrettjonesgoogle commented Dec 21, 2017

@neozwu would you be able to finish this up?

@neozwu
Copy link
Contributor Author

neozwu commented Dec 21, 2017

@garrettjonesgoogle should be ready to merge once API goes public

@garrettjonesgoogle garrettjonesgoogle added the status: blocked Resolving the issue is dependent on other work. label Dec 21, 2017
@garrettjonesgoogle
Copy link
Member

garrettjonesgoogle commented Dec 21, 2017

I added the "status: blocked" tag since this is blocked on the server side.

@neozwu neozwu requested a review from pongad as a code owner January 31, 2018 20:49
@neozwu neozwu removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. status: blocked Resolving the issue is dependent on other work. labels Jan 31, 2018
@neozwu neozwu merged commit 32a6648 into googleapis:master Jan 31, 2018
chingor13 pushed a commit that referenced this pull request Feb 20, 2026
@release-please
chore(main): release 3.8.2-SNAPSHOT (#2613)
9e3dfa5 
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
chingor13 pushed a commit that referenced this pull request Mar 24, 2026
@JoeWang1127
chore: add integration test running in docker container (#2613)
bbcf832 
In this PR:
- In the integration test, run `generate_repo.py` and
`generate_pr_description.py` in docker container.
- Remove test image in cloud build.

Context: 
In this
[log](https://github.com/googleapis/google-cloud-java/actions/runs/8445620169/job/23133097820),
the nightly generation job only took 3 mins. After investigation, we
found out that removing `main` method in `generate_repo.py` (#2598)
caused the CLI is not running despite that the job returns successfully.

This non-explicit failure demonstrated that there's difference between
calling CLI (`generate` method in `generate_repo.py`) and its
implementation (`generate_from_yaml` method in `generate_repo.py`).

We decided to change the integration test to run the CLI in docker
container, rather than testing `generate_from_yaml` because it is the
containerized CLI will be used in downstream libraries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pongad pongad Awaiting requested review from pongad

2 more reviewers

@garrettjonesgoogle garrettjonesgoogle garrettjonesgoogle left review comments

@vam-google vam-google vam-google approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

4 participants

@neozwu @garrettjonesgoogle @vam-google @googlebot