{"action":"created","comment":{"url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/comments/87597571","pull_request_review_id":8205966,"id":87597571,"diff_hunk":"@@ -151,6 +152,10 @@ def override_url_for():\n     application.config['THEME_PATHS'] = os.path.dirname(os.path.abspath(__file__))\n     Themes(application, app_identifier=\"surveyrunner\")\n \n+    @application.teardown_appcontext\n+    def shutdown_session(exception=None):    # pylint: disable=unused-argument","path":"app/__init__.py","position":13,"original_position":13,"commit_id":"7b2da65e58da3efbdf0ede277e8e01213185c943","original_commit_id":"7b2da65e58da3efbdf0ede277e8e01213185c943","user":{"login":"codacy-bot","id":19940114,"avatar_url":"https://avatars.githubusercontent.com/u/19940114?v=3","gravatar_id":"","url":"https://api.github.com/users/codacy-bot","html_url":"https://github.com/codacy-bot","followers_url":"https://api.github.com/users/codacy-bot/followers","following_url":"https://api.github.com/users/codacy-bot/following{/other_user}","gists_url":"https://api.github.com/users/codacy-bot/gists{/gist_id}","starred_url":"https://api.github.com/users/codacy-bot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/codacy-bot/subscriptions","organizations_url":"https://api.github.com/users/codacy-bot/orgs","repos_url":"https://api.github.com/users/codacy-bot/repos","events_url":"https://api.github.com/users/codacy-bot/events{/privacy}","received_events_url":"https://api.github.com/users/codacy-bot/received_events","type":"User","site_admin":false},"body":"![Codacy](https://www.codacy.com/assets/images/favicon.png) Issue found: [Unused variable 'shutdown_session'](https://www.codacy.com/app/ONSDigital/eq-survey-runner/file/4424162251/issues/source?bid=3899578&fileBranchId=3899608#l156)","created_at":"2016-11-11T14:41:34Z","updated_at":"2016-11-11T14:41:34Z","html_url":"https://github.com/ONSdigital/eq-survey-runner/pull/556#discussion_r87597571","pull_request_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/556","_links":{"self":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/comments/87597571"},"html":{"href":"https://github.com/ONSdigital/eq-survey-runner/pull/556#discussion_r87597571"},"pull_request":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/556"}}},"pull_request":{"url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/556","id":93191449,"html_url":"https://github.com/ONSdigital/eq-survey-runner/pull/556","diff_url":"https://github.com/ONSdigital/eq-survey-runner/pull/556.diff","patch_url":"https://github.com/ONSdigital/eq-survey-runner/pull/556.patch","issue_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues/556","number":556,"state":"open","locked":false,"title":"Fixing SQLAlchemy transaction handling","user":{"login":"benjefferies","id":15050019,"avatar_url":"https://avatars.githubusercontent.com/u/15050019?v=3","gravatar_id":"","url":"https://api.github.com/users/benjefferies","html_url":"https://github.com/benjefferies","followers_url":"https://api.github.com/users/benjefferies/followers","following_url":"https://api.github.com/users/benjefferies/following{/other_user}","gists_url":"https://api.github.com/users/benjefferies/gists{/gist_id}","starred_url":"https://api.github.com/users/benjefferies/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/benjefferies/subscriptions","organizations_url":"https://api.github.com/users/benjefferies/orgs","repos_url":"https://api.github.com/users/benjefferies/repos","events_url":"https://api.github.com/users/benjefferies/events{/privacy}","received_events_url":"https://api.github.com/users/benjefferies/received_events","type":"User","site_admin":false},"body":"### What is the context of this PR?\r\nDescribe what you have changed and why, link to other PRs or Issues as appropriate.\r\n\r\nWe saw a error in production which suggested a transaction had already\r\nbeen closed `sqlalchemy.exc.InvalidRequestError: This Session's\r\ntransaction has been rolled back due to a previous exception during\r\nflush`. On further investigation this was because of a duplicate\r\nquestionnaire PK when committing a transaction. Further down the line\r\nthe application tried to make use of the session which due to the error\r\nmeant we had no transaction to use as it was left in a dirty state\r\nwithout being rolled back. This caused a stacktrace to be displayed on\r\nsimulation.\r\n\r\n* Removing db_session when application dies and at the end of a request\r\n  as per [flask documentation]\r\n  (http://flask.pocoo.org/docs/0.11/patterns/sqlalchemy/#declarative).\r\n* Catching errors on commit and rolling back then re-raising error.\r\n* Removing unused interface\r\n* Logging tx_id against errors\r\n\r\nFurther reading: On investigating different transaction handling\r\nstrategies one suggestion which seems quite applicable in our scenario\r\nwas to map the transaction to the http request which makes sense when\r\nfollowing the `unit of work` pattern. The benefits of this would be we\r\nwouldn't have to manually handle our transactions for all of the\r\ndatabase code and we'd have one place to handle it thus one area to test\r\nregarding transactions.\r\n\r\n### How to review \r\nDescribe the steps required to test the changes (include screenshots if appropriate).\r\n\r\n#### Simulating duplicate PK (developer)\r\n\r\n1. Add threaded configuration as `Server` parameter at https://github.com/ONSdigital/eq-survey-runner/blob/master/application.py#L16. See https://arusahni.net/blog/2013/10/flask-multithreading.html\r\n1. Run application in debug with a breakpoint at https://github.com/ONSdigital/eq-survey-runner/blob/eq-494-fix-database-transaction-handling/app/storage/database_storage.py#L24\r\n1. Start a new survey as a new unique user\r\n1. You should hit the break point (Do not resume it yet)\r\n1. Open another tab and start another survey as the same user\r\n1. Resume both of the threads\r\n1. Observe 500 error instead of stacktrace","created_at":"2016-11-10T17:07:34Z","updated_at":"2016-11-11T14:41:34Z","closed_at":null,"merged_at":null,"merge_commit_sha":"3be4c8f86bd02c91493d858eadbcc20f28ea6380","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/556/commits","review_comments_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/556/comments","review_comment_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues/556/comments","statuses_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/statuses/7b2da65e58da3efbdf0ede277e8e01213185c943","head":{"label":"ONSdigital:eq-494-fix-database-transaction-handling","ref":"eq-494-fix-database-transaction-handling","sha":"7b2da65e58da3efbdf0ede277e8e01213185c943","user":{"login":"ONSdigital","id":6865137,"avatar_url":"https://avatars.githubusercontent.com/u/6865137?v=3","gravatar_id":"","url":"https://api.github.com/users/ONSdigital","html_url":"https://github.com/ONSdigital","followers_url":"https://api.github.com/users/ONSdigital/followers","following_url":"https://api.github.com/users/ONSdigital/following{/other_user}","gists_url":"https://api.github.com/users/ONSdigital/gists{/gist_id}","starred_url":"https://api.github.com/users/ONSdigital/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ONSdigital/subscriptions","organizations_url":"https://api.github.com/users/ONSdigital/orgs","repos_url":"https://api.github.com/users/ONSdigital/repos","events_url":"https://api.github.com/users/ONSdigital/events{/privacy}","received_events_url":"https://api.github.com/users/ONSdigital/received_events","type":"Organization","site_admin":false},"repo":{"id":49205589,"name":"eq-survey-runner","full_name":"ONSdigital/eq-survey-runner","owner":{"login":"ONSdigital","id":6865137,"avatar_url":"https://avatars.githubusercontent.com/u/6865137?v=3","gravatar_id":"","url":"https://api.github.com/users/ONSdigital","html_url":"https://github.com/ONSdigital","followers_url":"https://api.github.com/users/ONSdigital/followers","following_url":"https://api.github.com/users/ONSdigital/following{/other_user}","gists_url":"https://api.github.com/users/ONSdigital/gists{/gist_id}","starred_url":"https://api.github.com/users/ONSdigital/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ONSdigital/subscriptions","organizations_url":"https://api.github.com/users/ONSdigital/orgs","repos_url":"https://api.github.com/users/ONSdigital/repos","events_url":"https://api.github.com/users/ONSdigital/events{/privacy}","received_events_url":"https://api.github.com/users/ONSdigital/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ONSdigital/eq-survey-runner","description":"eQ Survey Runner","fork":false,"url":"https://api.github.com/repos/ONSdigital/eq-survey-runner","forks_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/forks","keys_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/teams","hooks_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/hooks","issue_events_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues/events{/number}","events_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/events","assignees_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/assignees{/user}","branches_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/branches{/branch}","tags_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/tags","blobs_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/refs{/sha}","trees_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/statuses/{sha}","languages_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/languages","stargazers_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/stargazers","contributors_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/contributors","subscribers_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/subscribers","subscription_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/subscription","commits_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/commits{/sha}","git_commits_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/commits{/sha}","comments_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/comments{/number}","issue_comment_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues/comments{/number}","contents_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/contents/{+path}","compare_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/merges","archive_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/downloads","issues_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues{/number}","pulls_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls{/number}","milestones_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/milestones{/number}","notifications_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/labels{/name}","releases_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/releases{/id}","deployments_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/deployments","created_at":"2016-01-07T13:15:26Z","updated_at":"2016-11-10T09:21:31Z","pushed_at":"2016-11-11T14:41:08Z","git_url":"git://github.com/ONSdigital/eq-survey-runner.git","ssh_url":"git@github.com:ONSdigital/eq-survey-runner.git","clone_url":"https://github.com/ONSdigital/eq-survey-runner.git","svn_url":"https://github.com/ONSdigital/eq-survey-runner","homepage":null,"size":7865,"stargazers_count":5,"watchers_count":5,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2,"mirror_url":null,"open_issues_count":12,"forks":2,"open_issues":12,"watchers":5,"default_branch":"master"}},"base":{"label":"ONSdigital:master","ref":"master","sha":"89c14af82f8f31e7e7d841a5444c4b414a0322ce","user":{"login":"ONSdigital","id":6865137,"avatar_url":"https://avatars.githubusercontent.com/u/6865137?v=3","gravatar_id":"","url":"https://api.github.com/users/ONSdigital","html_url":"https://github.com/ONSdigital","followers_url":"https://api.github.com/users/ONSdigital/followers","following_url":"https://api.github.com/users/ONSdigital/following{/other_user}","gists_url":"https://api.github.com/users/ONSdigital/gists{/gist_id}","starred_url":"https://api.github.com/users/ONSdigital/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ONSdigital/subscriptions","organizations_url":"https://api.github.com/users/ONSdigital/orgs","repos_url":"https://api.github.com/users/ONSdigital/repos","events_url":"https://api.github.com/users/ONSdigital/events{/privacy}","received_events_url":"https://api.github.com/users/ONSdigital/received_events","type":"Organization","site_admin":false},"repo":{"id":49205589,"name":"eq-survey-runner","full_name":"ONSdigital/eq-survey-runner","owner":{"login":"ONSdigital","id":6865137,"avatar_url":"https://avatars.githubusercontent.com/u/6865137?v=3","gravatar_id":"","url":"https://api.github.com/users/ONSdigital","html_url":"https://github.com/ONSdigital","followers_url":"https://api.github.com/users/ONSdigital/followers","following_url":"https://api.github.com/users/ONSdigital/following{/other_user}","gists_url":"https://api.github.com/users/ONSdigital/gists{/gist_id}","starred_url":"https://api.github.com/users/ONSdigital/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ONSdigital/subscriptions","organizations_url":"https://api.github.com/users/ONSdigital/orgs","repos_url":"https://api.github.com/users/ONSdigital/repos","events_url":"https://api.github.com/users/ONSdigital/events{/privacy}","received_events_url":"https://api.github.com/users/ONSdigital/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ONSdigital/eq-survey-runner","description":"eQ Survey Runner","fork":false,"url":"https://api.github.com/repos/ONSdigital/eq-survey-runner","forks_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/forks","keys_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/teams","hooks_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/hooks","issue_events_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues/events{/number}","events_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/events","assignees_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/assignees{/user}","branches_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/branches{/branch}","tags_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/tags","blobs_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/refs{/sha}","trees_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/statuses/{sha}","languages_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/languages","stargazers_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/stargazers","contributors_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/contributors","subscribers_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/subscribers","subscription_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/subscription","commits_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/commits{/sha}","git_commits_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/git/commits{/sha}","comments_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/comments{/number}","issue_comment_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues/comments{/number}","contents_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/contents/{+path}","compare_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/merges","archive_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/downloads","issues_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues{/number}","pulls_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls{/number}","milestones_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/milestones{/number}","notifications_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/labels{/name}","releases_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/releases{/id}","deployments_url":"https://api.github.com/repos/ONSdigital/eq-survey-runner/deployments","created_at":"2016-01-07T13:15:26Z","updated_at":"2016-11-10T09:21:31Z","pushed_at":"2016-11-11T14:41:08Z","git_url":"git://github.com/ONSdigital/eq-survey-runner.git","ssh_url":"git@github.com:ONSdigital/eq-survey-runner.git","clone_url":"https://github.com/ONSdigital/eq-survey-runner.git","svn_url":"https://github.com/ONSdigital/eq-survey-runner","homepage":null,"size":7865,"stargazers_count":5,"watchers_count":5,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2,"mirror_url":null,"open_issues_count":12,"forks":2,"open_issues":12,"watchers":5,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/556"},"html":{"href":"https://github.com/ONSdigital/eq-survey-runner/pull/556"},"issue":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues/556"},"comments":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/issues/556/comments"},"review_comments":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/556/comments"},"review_comment":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/pulls/556/commits"},"statuses":{"href":"https://api.github.com/repos/ONSdigital/eq-survey-runner/statuses/7b2da65e58da3efbdf0ede277e8e01213185c943"}}}}
{"actor":{"display_login":"codacy-bot"}}