Skip to content

Adding system test for datastore failed transactions. #1055

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 12, 2015
Merged

Adding system test for datastore failed transactions. #1055

merged 1 commit into from
Aug 12, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Aug 11, 2015

This intentionally forces data contention in a transaction (by updating the entity outside of the transaction).

This is to prevent regressions like #903.

@dhermes dhermes added api: datastore Issues related to the Datastore API. testing labels Aug 11, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 11, 2015
@dhermes dhermes mentioned this pull request Aug 11, 2015
Closed
tseaver
tseaver reviewed Aug 12, 2015
View reviewed changes
system_tests/datastore.py
with self.assertRaises(Conflict):
txn.commit()

CLIENT._pop_batch()

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Adding system test for datastore failed transactions.
d04da8a 
This intentionally forces data contention in a transaction (by
updating the entity outside of the transaction).

This is to prevent regressions like googleapis#903.
@dhermes dhermes force-pushed the force-failed-transaction branch from 41230aa to d04da8a Compare August 12, 2015 18:46
@dhermes
Copy link
Contributor Author

dhermes commented Aug 12, 2015

@tseaver I took a different approach, by creating a local client and using the global CLIENT to make the conflicting change during the transaction. PTAL.

@tseaver
Copy link
Contributor

tseaver commented Aug 12, 2015

LGTM. That reads much better, too.

dhermes added a commit that referenced this pull request Aug 12, 2015
@dhermes
Merge pull request #1055 from dhermes/force-failed-transaction
dea61ad 
Adding system test for datastore failed transactions.
@dhermes dhermes merged commit dea61ad into googleapis:master Aug 12, 2015
@dhermes dhermes deleted the force-failed-transaction branch August 12, 2015 20:41
@dhermes
Copy link
Contributor Author

dhermes commented Aug 12, 2015

@stephenplusplus @callmehiphop @ryanseys FYI we implemented a system test to make sure that a transaction would fail if we messed with the values outside of (but during) the transaction.

This would have caught a serious regression we had (a month or two ago) where transaction IDs weren't being sent in requests.