datastore: fix - mark transactions as finalized. #250

stephenplusplus · 2014-10-08T20:41:44Z

The recent API refactoring removed the functionality where save and delete operations mark the transaction as finalized. This brings it back.

ryanseys · 2014-10-08T22:42:19Z

Oops. Do we have to wrap the method or can we detect that it's a transaction by looking for this.id?

stephenplusplus · 2014-10-08T22:44:20Z

Oh, good call. We can finalize from datastore request if you think that's
clearer

On Wednesday, October 8, 2014, Ryan Seys notifications@github.com wrote:

Oops. Do we have to wrap the method or can we detect that it's a
transaction by looking for this.id?

—
Reply to this email directly or view it on GitHub
#250 (comment)
.

ryanseys · 2014-10-08T22:44:59Z

I'd prefer all the logic to be there, yes. Much easier to follow it that way.

stephenplusplus · 2014-10-09T00:14:54Z

Updated with the new plan.

lib/datastore/request.js

-  this.createRequest_('commit', req, res, callback);
+  this.createRequest_('commit', req, res, function(err) {
+    if (!err && this.id) {
+      this.isFinalized = true;


ryanseys · 2014-10-09T03:42:07Z

Just worried if the transaction isn't finalized when a response is received, that could have unexpected side-effects. Other than that, this looks good to me :)

silvolu · 2014-10-09T17:06:27Z

Can we test it somehow? Having a test would have avoided the regression.

stephenplusplus · 2014-10-09T17:11:03Z

@silvolu I had the same thought. I figured I'd let this PR go, then attack a re-factor of datastore tests and add it at that time, as well as anything else that's missing. After we put the shared logic into request.js, our test files aren't split to match, so functionality testing is a bit criss-crossed atm.

silvolu · 2014-10-09T17:13:28Z

Ok, I'll merge this, then let's wait for the tests to be fixed before releasing 0.8.1

…transactions datastore: fix - mark transactions as finalized.

ryanseys · 2014-10-09T17:14:54Z

The tests aren't failing so curious what "vector" you're going to take to approach fixing them? Asking for a friend 😉

silvolu · 2014-10-09T17:17:01Z

Sorry, poor choice of word :)
'Fixing' as in split the files to reflect the new structure and add coverage for functionalities that are not tested (like this one).

ryanseys · 2014-10-09T17:22:39Z

Makes sense. My apologies for assuming they were sufficient as-is :) Let me know if I can help with the transition.

BREAKING CHANGE: removes projectPath helper, instead use "projects/${project}".

* updated CHANGELOG.md * updated package.json * updated samples/package.json

Fixes #234 Fixes #250 Addressed comments.

…ript generator. (#250) Also removing the explicit generator tag for the IAMPolicy mixin for the kms and pubsub APIS as the generator will now read it from the .yaml file. PiperOrigin-RevId: 385101839 Source-Link: googleapis/googleapis@80f4042 Source-Link: googleapis/googleapis-gen@d3509d2

@alexander-fenster

The library is regenerated with gapic-generator-typescript v1.3.1. Committer: @alexander-fenster PiperOrigin-RevId: 372468161 Source-Link: googleapis/googleapis@75880c3 Source-Link: googleapis/googleapis-gen@77b1804

- [ ] Regenerate this pull request now. PiperOrigin-RevId: 474338479 Source-Link: googleapis/googleapis@d5d35e0 Source-Link: googleapis/googleapis-gen@efcd3f9 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZWZjZDNmOTM5NjJhMTAzZjY4ZjAwM2UyYTFlZWNkZTZmYTIxNmEyNyJ9

* chore(main): release 3.0.3 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

BREAKING CHANGE: removes projectPath helper, instead use "projects/${project}".

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/35a31f24-4249-4b7d-9c51-7f63c556390c/targets - [ ] To automatically regenerate this PR, check this box. Source-Link: googleapis/synthtool@0c868d4

…250)

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

…250)

stephenplusplus added in progress and removed in progress labels Oct 8, 2014

stephenplusplus mentioned this pull request Oct 8, 2014

modularizing server and introduce cli example. GoogleCloudPlatform/google-cloud-node-todos#6

Merged

stephenplusplus force-pushed the spp--datastore-finalize-transactions branch 2 times, most recently from e59a61f to cc519d4 Compare October 9, 2014 00:14

datastore: fix - mark transactions as finalized.

c79a52d

stephenplusplus force-pushed the spp--datastore-finalize-transactions branch from cc519d4 to c79a52d Compare October 9, 2014 00:14

ryanseys reviewed Oct 9, 2014
View reviewed changes

lib/datastore/request.js

this.createRequest_('commit', req, res, callback);

this.createRequest_('commit', req, res, function(err) {

if (!err && this.id) {

this.isFinalized = true;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

silvolu added a commit that referenced this pull request Oct 9, 2014

Merge pull request #250 from stephenplusplus/spp--datastore-finalize-…

c528eed

…transactions datastore: fix - mark transactions as finalized.

silvolu merged commit c528eed into googleapis:master Oct 9, 2014

stephenplusplus mentioned this pull request Oct 9, 2014

datastore: refactor tests. #252

Merged

chingor13 pushed a commit that referenced this pull request Aug 22, 2022

feat!: converts library to TypeScript adding v1p1beta1 surface (#250)

30a105e

BREAKING CHANGE: removes projectPath helper, instead use "projects/${project}".

sofisl mentioned this pull request Sep 15, 2022

migrate code from googleapis/nodejs-redis #3360

Merged

4 tasks

sofisl pushed a commit that referenced this pull request Sep 15, 2022

chore(deps): update dependency c8 to v7 (#250)

d486ba9

sofisl pushed a commit that referenced this pull request Sep 27, 2022

chore: release 2.1.0 (#250)

b4f67c5

* updated CHANGELOG.md * updated package.json * updated samples/package.json

sofisl mentioned this pull request Sep 27, 2022

migrate code from googleapis/nodejs-video-intelligence #3374

Closed

4 tasks

sofisl pushed a commit that referenced this pull request Oct 11, 2022

Fixes #48 (#249)

8c024c5

Fixes #234 Fixes #250 Addressed comments.

sofisl mentioned this pull request Oct 11, 2022

migrate code from googleapis/nodejs-language #3395

Merged

4 tasks

sofisl mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-web-risk #3549

Merged

4 tasks

This was referenced Nov 11, 2022

migrate code from googleapis/nodejs-billing-budgets #3552

Merged

migrate code from googleapis/nodejs-secret-manager #3553

Merged

migrate code from googleapis/nodejs-service-directory #3557

Merged

sofisl mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-media-translation #3558

Merged

4 tasks

sofisl mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-document-ai #3559

Merged

4 tasks

sofisl pushed a commit that referenced this pull request Nov 11, 2022

chore(nodejs): update client ref docs link in metadata (#250)

0dcf2b3

sofisl pushed a commit that referenced this pull request Nov 11, 2022

feat!: converts library to TypeScript adding v1p1beta1 surface (#250)

3b5d06c

BREAKING CHANGE: removes projectPath helper, instead use "projects/${project}".

sofisl mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-cloudbuild #3566

Merged

4 tasks

sofisl pushed a commit that referenced this pull request Nov 11, 2022

chore: release 2.2.6 (#250)

ea24b6e

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

This was referenced Nov 11, 2022

migrate code from googleapis/nodejs-asset #3567

Merged

migrate code from googleapis/nodejs-talent #3568

Merged

sofisl mentioned this pull request Nov 18, 2022

migrate code from googleapis/nodejs-security-center #3669

Merged

4 tasks

sofisl pushed a commit that referenced this pull request Nov 18, 2022

build: set AUTOSYNTH_MULTIPLE_COMMITS=true for context aware commits (#…

9f72bed

…250)

sofisl mentioned this pull request Nov 30, 2022

migrate code from googleapis/nodejs-datacatalog #3689

Merged

4 tasks

sofisl pushed a commit that referenced this pull request Nov 30, 2022

chore: release 2.3.1 (#250)

ad5ca9d

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

sofisl pushed a commit that referenced this pull request Jan 24, 2023

chore: update license file (#250)

88edb73

sofisl pushed a commit that referenced this pull request Jan 25, 2023

chore: update license file (#250)

25ba243

sofisl pushed a commit that referenced this pull request Jan 26, 2023

docs(badges): tweak badge to use new preview/stable language (#1314) (#…

188c3a8

…250)

sofisl mentioned this pull request Jan 26, 2023

migrate code from googleapis/nodejs-ai-platform #3892

Merged

4 tasks

sofisl mentioned this pull request Sep 13, 2023

migrate code from googleapis/nodejs-automl #4655

Closed

4 tasks

sofisl pushed a commit that referenced this pull request Sep 13, 2023

chore: update CONTRIBUTING.md and make releaseType node (#250)

592dd77

sofisl mentioned this pull request Sep 14, 2023

migrate code from googleapis/nodejs-automl #4656

Merged

4 tasks

sofisl pushed a commit that referenced this pull request Sep 14, 2023

fix(deps): update dependency @google-cloud/paginator to v1 (#250)

02b25f8

sofisl mentioned this pull request Sep 14, 2023

migrate code from googleapis/nodejs-dns #4657

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

datastore: fix - mark transactions as finalized. #250

datastore: fix - mark transactions as finalized. #250

stephenplusplus commented Oct 8, 2014

ryanseys commented Oct 8, 2014

stephenplusplus commented Oct 8, 2014

ryanseys commented Oct 8, 2014

stephenplusplus commented Oct 9, 2014

This comment was marked as spam.

This comment was marked as spam.

ryanseys commented Oct 9, 2014

silvolu commented Oct 9, 2014

stephenplusplus commented Oct 9, 2014

silvolu commented Oct 9, 2014

ryanseys commented Oct 9, 2014

silvolu commented Oct 9, 2014

ryanseys commented Oct 9, 2014

datastore: fix - mark transactions as finalized. #250

datastore: fix - mark transactions as finalized. #250

Conversation

stephenplusplus commented Oct 8, 2014

ryanseys commented Oct 8, 2014

stephenplusplus commented Oct 8, 2014

ryanseys commented Oct 8, 2014

stephenplusplus commented Oct 9, 2014

This comment was marked as spam.

This comment was marked as spam.

ryanseys commented Oct 9, 2014

silvolu commented Oct 9, 2014

stephenplusplus commented Oct 9, 2014

silvolu commented Oct 9, 2014

ryanseys commented Oct 9, 2014

silvolu commented Oct 9, 2014

ryanseys commented Oct 9, 2014