{"action":"created","issue":{"url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26","repository_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions","labels_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26/labels{/name}","comments_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26/comments","events_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26/events","html_url":"https://github.com/postmanlabs/sails-mysql-transactions/issues/26","id":123875228,"number":26,"title":"[reflection] [idea]","user":{"login":"mikermcneil","id":618009,"avatar_url":"https://avatars.githubusercontent.com/u/618009?v=3","gravatar_id":"","url":"https://api.github.com/users/mikermcneil","html_url":"https://github.com/mikermcneil","followers_url":"https://api.github.com/users/mikermcneil/followers","following_url":"https://api.github.com/users/mikermcneil/following{/other_user}","gists_url":"https://api.github.com/users/mikermcneil/gists{/gist_id}","starred_url":"https://api.github.com/users/mikermcneil/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mikermcneil/subscriptions","organizations_url":"https://api.github.com/users/mikermcneil/orgs","repos_url":"https://api.github.com/users/mikermcneil/repos","events_url":"https://api.github.com/users/mikermcneil/events{/privacy}","received_events_url":"https://api.github.com/users/mikermcneil/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":22,"created_at":"2015-12-25T11:42:19Z","updated_at":"2016-02-28T11:26:15Z","closed_at":null,"body":"First of all, this module is awesome; amazing work (particularly the depth you went to w/ making instance methods do their thing when amidst a transaction).\r\n\r\nI wanted to leave a note here with some reflections on the history and future of transactions in WL core, and also share an idea for some syntactic sugar based on what I learned from WL2.\r\n\r\n### Transactions in WL core\r\n\r\nBad news first:  We will probably never have true atomic, durable, consistent, and isolated _cross-adapter transactions_ in Waterline core.  \r\n\r\n> Aside: I've always **dreamed** of having cross-adapter transactions in WL core.  I spent a bunch of time messing with this [waaaaaay back](https://github.com/balderdashy/sails/commits/master?page=126) when I was first building Waterline.  In fact, that was almost exactly 3 years ago to the day (Christmas 2012).  As it turns out, trying to implement cross-adapter transactions is a [nightmare](https://github.com/balderdashy/sails/blob/09037937366cecc222c7a53829d3c6e4f64ad822/adapter.js#L307) and basically like [writing your own database](https://github.com/mikermcneil/waterline2/blob/master/Introduction%20to%20Waterline2.pdf). Nevertheless, for some reason I spent the time to get the \"CI\" in \"ACID\" working-- but I never got around to implementing inverse operations in order to be able to roll back-- which we would need to form a truly crash-resistant (read \"durable\") [commit log](https://github.com/balderdashy/sails/blob/09037937366cecc222c7a53829d3c6e4f64ad822/collections/Transaction.js)).  Long story short: it is an interesting exercise, but ultimately a bad idea.\r\n\r\nThe good news is that we will _definitely_ implement transactions across the same connection to the same database.  The question is just when-- and you guys have done an amazing job hastening that along.\r\n\r\n\r\n### An idea re: syntax\r\n\r\nThe #1 thing I've always been worried about w/ transactions is forgetting to rollback under rare error conditions (i.e. that never get tested).  The #2 thing has always been forgetting to use the proper transaction instance and accidentally using models directly.\r\n\r\nWhen I was actively working on WL2 last summer, the simplest possible usage I could come up with which also addressed both issues (at least to some degree) was something like [this](https://github.com/mikermcneil/waterline2/blob/16ac1edf7e113eee94970ce5ba894b0d0eddbb0a/lib/ORM/ORM.prototype.transaction.js#L17):\r\n\r\n```javascript\r\norm.transaction([User, Kitten], function (User, Kitten, cb) {\r\n  Kitten.create({ name: req.param('kittenName') }).exec(function (err, babyKitten){\r\n    if (err) {\r\n      return cb(err); // << that means rollback\r\n    }\r\n    User.update({ id: req.session.userId }).set({ petKitten: babyKitten.id }).exec(function (err, updatedUsers) {\r\n      if (err)  {\r\n        return cb(err);  // << that means rollback\r\n      }\r\n      if (updatedUsers.length === 0) {\r\n        err = new Error('Consistency violation: Could no longer find currently logged in user in database!');\r\n        err.code = 'loggedInUserMissingFromDb';\r\n        return cb(err);  // << that means rollback\r\n      }\r\n      return cb(undefined, updatedUsers[0]);  // << that means commit  (so would `cb()`)\r\n      // (second arg is optional, but if you pass it in, it'll show up as the 2nd argument\r\n      //  of the callback below)\r\n    });\r\n  });\r\n}).exec(function (err, data) {\r\n  if (err) {\r\n    // if `err` is truthy, then there was an error, and the transaction was\r\n    // already rolled back by the time we make it here.\r\n    return res.negotiate(err);\r\n  }\r\n\r\n  // if not, there were no errors, and the transaction was safely committed\r\n  // by the time we make it here!\r\n  return res.json({\r\n    username: data.username,\r\n    kittenId: data.petKitten\r\n  });\r\n});\r\n```\r\n\r\nBelow, I put together a hypothetical remapping of the example in the README of this module if it was wrapped up in something like this (in _addition_ to `.start()`, not instead of it).  Please take it with a grain of salt-- I'm not saying I think it should be this way in _this_ module necessarily (it's more in-line with what we'd need in core Waterline anyway-- i.e.  triggering a usage error if the models are not part of the same database connection).  I just want to make sure and share this thinking with you guys in case its helpful or spurs any ideas for folding transaction support into Sails/Waterline core.  \r\n\r\nHere goes nothing:\r\n\r\n```javascript\r\nvar Transaction = require('sails-mysql-transactions').Transaction;\r\n\r\n// Attempt a database transaction\r\nTransaction.attempt([OneModel, AnotherModel], function (OneModel, AnotherModel, cb) {\r\n  OneModel.create(req.params.all(), function (err, someRecord) {\r\n    if (err) {\r\n      return cb(err);\r\n    }\r\n    AnotherModel.findOne(req.param('id')).exec(function (err, anotherRecord) {\r\n      if (err) {\r\n        return cb(err);\r\n      }\r\n      someRecord.someAssociation.remove(req.param('remove_id'));\r\n      someRecord.save(function (err, savedRecord){\r\n        if (err) {\r\n          return cb(err);\r\n        }\r\n        return cb({\r\n          savedRecord: savedRecord\r\n          anotherRecord: anotherRecord\r\n        });\r\n      });\r\n    });\r\n  });\r\n}).exec(function (err, data) {\r\n  if (err) {\r\n    // This means that either something explicitly passed back an error in the provided function above,\r\n    // or the function never ran because a transaction could not be started, or that the transaction was\r\n    // otherwise successful but could not be committed. If we're here, we know the transaction has\r\n    // been rolled back successfully.\r\n    return res.serverError(err);\r\n  }\r\n  \r\n  // Otherwise, we know everything worked and that the transaction was committed.\r\n  return res.json({\r\n    one: data.savedModel,\r\n    another: data.anotherInstance\r\n  });\r\n});\r\n```\r\n\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/comments/189848165","html_url":"https://github.com/postmanlabs/sails-mysql-transactions/issues/26#issuecomment-189848165","issue_url":"https://api.github.com/repos/postmanlabs/sails-mysql-transactions/issues/26","id":189848165,"user":{"login":"shamasis","id":232373,"avatar_url":"https://avatars.githubusercontent.com/u/232373?v=3","gravatar_id":"","url":"https://api.github.com/users/shamasis","html_url":"https://github.com/shamasis","followers_url":"https://api.github.com/users/shamasis/followers","following_url":"https://api.github.com/users/shamasis/following{/other_user}","gists_url":"https://api.github.com/users/shamasis/gists{/gist_id}","starred_url":"https://api.github.com/users/shamasis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shamasis/subscriptions","organizations_url":"https://api.github.com/users/shamasis/orgs","repos_url":"https://api.github.com/users/shamasis/repos","events_url":"https://api.github.com/users/shamasis/events{/privacy}","received_events_url":"https://api.github.com/users/shamasis/received_events","type":"User","site_admin":false},"created_at":"2016-02-28T11:26:15Z","updated_at":"2016-02-28T11:26:15Z","body":"@sapnajindal - changing the adapter is simply changing the name of the adapter in connections.js, followed by updating package.json to install this adapter. \r\n\r\nWe're using this adapter in production under real heavy load. Also, note that SailsJS is also planning to introduce native transaction support in waterline very soon."}}