{"action":"created","comment":{"url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/comments/129051890","pull_request_review_id":51786613,"id":129051890,"diff_hunk":"@@ -52,10 +53,35 @@ def attach_bucket_policy(event, context):\n     team_slug = event[\"team\"][\"slug\"]\n \n     client = boto3.client(\"iam\")\n-    client.attach_role_policy(\n-        RoleName=naming.role_name(username),\n-        PolicyArn=policy_arn(team_slug, policy_type),\n-    )\n+\n+    def fn():\n+        client.attach_role_policy(\n+            RoleName=naming.role_name(username),\n+            PolicyArn=policy_arn(team_slug, policy_type),\n+        )\n+\n+    retry(fn)\n+\n+\n+def retry(fn, attempts=5, delay=0.150):\n+    tries = attempts\n+    while tries > 0:\n+        try:\n+            fn()\n+            break\n+        except botocore.exceptions.ClientError as error:\n+            if error.response[\"Error\"][\"Code\"] != \"NoSuchEntity\":\n+                # Only retry on boto's ClientError/NoSuchEntity error\n+                raise\n+\n+            tries -= 1\n+            LOG.warning(\n+                \"error while attaching role to policy: {}.\".format(error))\n+            if tries > 0:\n+                LOG.warning(\"Retry in {}s...\".format(delay))\n+                time.sleep(delay)","path":"infra/terraform/modules/data_access/memberships/memberships.py","position":42,"original_position":42,"commit_id":"8ac4cafcfefc1bd656de2d49760628c7c5a3b13c","original_commit_id":"8ac4cafcfefc1bd656de2d49760628c7c5a3b13c","user":{"login":"andyhd","id":35194,"avatar_url":"https://avatars3.githubusercontent.com/u/35194?v=4","gravatar_id":"","url":"https://api.github.com/users/andyhd","html_url":"https://github.com/andyhd","followers_url":"https://api.github.com/users/andyhd/followers","following_url":"https://api.github.com/users/andyhd/following{/other_user}","gists_url":"https://api.github.com/users/andyhd/gists{/gist_id}","starred_url":"https://api.github.com/users/andyhd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andyhd/subscriptions","organizations_url":"https://api.github.com/users/andyhd/orgs","repos_url":"https://api.github.com/users/andyhd/repos","events_url":"https://api.github.com/users/andyhd/events{/privacy}","received_events_url":"https://api.github.com/users/andyhd/received_events","type":"User","site_admin":false},"body":"I checked, you don't need `break`:\r\n```python\r\n>>> x = 0\r\n>>> while True:\r\n...     try:\r\n...         raise Exception('foo')\r\n...     except Exception as e:\r\n...         x += 1\r\n...         if x < 5:\r\n...             print('retry', x)\r\n...             continue\r\n...         raise\r\n...\r\nretry 1\r\nretry 2\r\nretry 3\r\nretry 4\r\nTraceback (most recent call last):\r\n  File \"<stdin>\", line 3, in <module>\r\nException: foo\r\n```","created_at":"2017-07-24T14:30:59Z","updated_at":"2017-07-24T14:30:59Z","html_url":"https://github.com/ministryofjustice/analytics-platform-ops/pull/89#discussion_r129051890","pull_request_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/89","_links":{"self":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/comments/129051890"},"html":{"href":"https://github.com/ministryofjustice/analytics-platform-ops/pull/89#discussion_r129051890"},"pull_request":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/89"}}},"pull_request":{"url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/89","id":130422417,"html_url":"https://github.com/ministryofjustice/analytics-platform-ops/pull/89","diff_url":"https://github.com/ministryofjustice/analytics-platform-ops/pull/89.diff","patch_url":"https://github.com/ministryofjustice/analytics-platform-ops/pull/89.patch","issue_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues/89","number":89,"state":"open","locked":false,"title":"attach_bucket_policy lambda function retries twice before notifying Sentry","user":{"login":"xoen","id":238563,"avatar_url":"https://avatars0.githubusercontent.com/u/238563?v=4","gravatar_id":"","url":"https://api.github.com/users/xoen","html_url":"https://github.com/xoen","followers_url":"https://api.github.com/users/xoen/followers","following_url":"https://api.github.com/users/xoen/following{/other_user}","gists_url":"https://api.github.com/users/xoen/gists{/gist_id}","starred_url":"https://api.github.com/users/xoen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/xoen/subscriptions","organizations_url":"https://api.github.com/users/xoen/orgs","repos_url":"https://api.github.com/users/xoen/repos","events_url":"https://api.github.com/users/xoen/events{/privacy}","received_events_url":"https://api.github.com/users/xoen/received_events","type":"User","site_admin":false},"body":"## What (NOW)\r\n\r\nWe now catch this specific exception and retry twice after a brief amount of\r\ntime (200ms, tested). This prevent the lambda failing and the error to get\r\nto us unless can't be handled this way.\r\n\r\n## Why (BEFORE)\r\n\r\nBecause of the asyncronous nature of these lambda functions, when creating\r\na new team or the user is new to the organisation:\r\n1) The team bucket's IAM policy may not be created yet\r\n2) The user's IAM role may not be created yet\r\n\r\nIn these two cases the `attach_bucket_policy` lambda function would raise\r\nan [`NoSuchEntity` exception](https://sentry.service.dsd.io/mojds/data-access-lambda/issues/5251/events/146927/).\r\nAWS will automatically retry and succeed but we'll be notified about this\r\nerror via Sentry.\r\n\r\n## Benefits\r\n\r\nAdd a retry/sleep is not the pretties thing but:\r\n1) From my manual experiments I've noticed that sleeping and retrying actually\r\n   improve the overall execution time. This is because not retrying will\r\n   cause a HTTP request to Sentry which increases the total execution time\r\n   (sometimes ~1000-1100ms), plus the execution time on the AWS retry (~700ms).\r\n   Conversely, sleep 200ms would cause the lambda function to succeed and\r\n   avoid the HTTP call to Sentry and the AWS retry bringing the total execution\r\n   time when this exception happen to ~1300ms (from 1700ms = 1000ms + 700ms)\r\n2) We have a very low number of invocation and we don't exceed the free tier\r\n3) Have false positive sentry errors lead to ignore the actual errors\r\n\r\n## Ticket\r\n\r\nhttps://trello.com/c/3VbPHbM9/273-retry-mechanism-for-dataaccess-lambdas","created_at":"2017-07-13T17:12:59Z","updated_at":"2017-07-24T14:30:59Z","closed_at":null,"merged_at":null,"merge_commit_sha":"d1ba71ce8263bf11d594f71a276279974d74c53a","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/89/commits","review_comments_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/89/comments","review_comment_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues/89/comments","statuses_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/statuses/8ac4cafcfefc1bd656de2d49760628c7c5a3b13c","head":{"label":"ministryofjustice:ag/data-access-lambda-retry","ref":"ag/data-access-lambda-retry","sha":"8ac4cafcfefc1bd656de2d49760628c7c5a3b13c","user":{"login":"ministryofjustice","id":2203574,"avatar_url":"https://avatars3.githubusercontent.com/u/2203574?v=4","gravatar_id":"","url":"https://api.github.com/users/ministryofjustice","html_url":"https://github.com/ministryofjustice","followers_url":"https://api.github.com/users/ministryofjustice/followers","following_url":"https://api.github.com/users/ministryofjustice/following{/other_user}","gists_url":"https://api.github.com/users/ministryofjustice/gists{/gist_id}","starred_url":"https://api.github.com/users/ministryofjustice/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ministryofjustice/subscriptions","organizations_url":"https://api.github.com/users/ministryofjustice/orgs","repos_url":"https://api.github.com/users/ministryofjustice/repos","events_url":"https://api.github.com/users/ministryofjustice/events{/privacy}","received_events_url":"https://api.github.com/users/ministryofjustice/received_events","type":"Organization","site_admin":false},"repo":{"id":77397761,"name":"analytics-platform-ops","full_name":"ministryofjustice/analytics-platform-ops","owner":{"login":"ministryofjustice","id":2203574,"avatar_url":"https://avatars3.githubusercontent.com/u/2203574?v=4","gravatar_id":"","url":"https://api.github.com/users/ministryofjustice","html_url":"https://github.com/ministryofjustice","followers_url":"https://api.github.com/users/ministryofjustice/followers","following_url":"https://api.github.com/users/ministryofjustice/following{/other_user}","gists_url":"https://api.github.com/users/ministryofjustice/gists{/gist_id}","starred_url":"https://api.github.com/users/ministryofjustice/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ministryofjustice/subscriptions","organizations_url":"https://api.github.com/users/ministryofjustice/orgs","repos_url":"https://api.github.com/users/ministryofjustice/repos","events_url":"https://api.github.com/users/ministryofjustice/events{/privacy}","received_events_url":"https://api.github.com/users/ministryofjustice/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ministryofjustice/analytics-platform-ops","description":"Ops and deployment resources for MOJ Analytics platform","fork":false,"url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops","forks_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/forks","keys_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/teams","hooks_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/hooks","issue_events_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues/events{/number}","events_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/events","assignees_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/assignees{/user}","branches_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/branches{/branch}","tags_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/tags","blobs_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/refs{/sha}","trees_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/statuses/{sha}","languages_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/languages","stargazers_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/stargazers","contributors_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/contributors","subscribers_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/subscribers","subscription_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/subscription","commits_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/commits{/sha}","git_commits_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/commits{/sha}","comments_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/comments{/number}","issue_comment_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues/comments{/number}","contents_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/contents/{+path}","compare_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/merges","archive_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/downloads","issues_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues{/number}","pulls_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls{/number}","milestones_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/milestones{/number}","notifications_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/labels{/name}","releases_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/releases{/id}","deployments_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/deployments","created_at":"2016-12-26T17:27:30Z","updated_at":"2017-07-24T08:46:19Z","pushed_at":"2017-07-24T10:56:50Z","git_url":"git://github.com/ministryofjustice/analytics-platform-ops.git","ssh_url":"git@github.com:ministryofjustice/analytics-platform-ops.git","clone_url":"https://github.com/ministryofjustice/analytics-platform-ops.git","svn_url":"https://github.com/ministryofjustice/analytics-platform-ops","homepage":"","size":1610,"stargazers_count":6,"watchers_count":6,"language":"HCL","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":5,"forks":1,"open_issues":5,"watchers":6,"default_branch":"master"}},"base":{"label":"ministryofjustice:master","ref":"master","sha":"ef1f0bf9d7f16ffecd66d5f22d5b3993b3d3110d","user":{"login":"ministryofjustice","id":2203574,"avatar_url":"https://avatars3.githubusercontent.com/u/2203574?v=4","gravatar_id":"","url":"https://api.github.com/users/ministryofjustice","html_url":"https://github.com/ministryofjustice","followers_url":"https://api.github.com/users/ministryofjustice/followers","following_url":"https://api.github.com/users/ministryofjustice/following{/other_user}","gists_url":"https://api.github.com/users/ministryofjustice/gists{/gist_id}","starred_url":"https://api.github.com/users/ministryofjustice/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ministryofjustice/subscriptions","organizations_url":"https://api.github.com/users/ministryofjustice/orgs","repos_url":"https://api.github.com/users/ministryofjustice/repos","events_url":"https://api.github.com/users/ministryofjustice/events{/privacy}","received_events_url":"https://api.github.com/users/ministryofjustice/received_events","type":"Organization","site_admin":false},"repo":{"id":77397761,"name":"analytics-platform-ops","full_name":"ministryofjustice/analytics-platform-ops","owner":{"login":"ministryofjustice","id":2203574,"avatar_url":"https://avatars3.githubusercontent.com/u/2203574?v=4","gravatar_id":"","url":"https://api.github.com/users/ministryofjustice","html_url":"https://github.com/ministryofjustice","followers_url":"https://api.github.com/users/ministryofjustice/followers","following_url":"https://api.github.com/users/ministryofjustice/following{/other_user}","gists_url":"https://api.github.com/users/ministryofjustice/gists{/gist_id}","starred_url":"https://api.github.com/users/ministryofjustice/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ministryofjustice/subscriptions","organizations_url":"https://api.github.com/users/ministryofjustice/orgs","repos_url":"https://api.github.com/users/ministryofjustice/repos","events_url":"https://api.github.com/users/ministryofjustice/events{/privacy}","received_events_url":"https://api.github.com/users/ministryofjustice/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ministryofjustice/analytics-platform-ops","description":"Ops and deployment resources for MOJ Analytics platform","fork":false,"url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops","forks_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/forks","keys_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/teams","hooks_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/hooks","issue_events_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues/events{/number}","events_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/events","assignees_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/assignees{/user}","branches_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/branches{/branch}","tags_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/tags","blobs_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/refs{/sha}","trees_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/statuses/{sha}","languages_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/languages","stargazers_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/stargazers","contributors_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/contributors","subscribers_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/subscribers","subscription_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/subscription","commits_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/commits{/sha}","git_commits_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/git/commits{/sha}","comments_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/comments{/number}","issue_comment_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues/comments{/number}","contents_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/contents/{+path}","compare_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/merges","archive_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/downloads","issues_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues{/number}","pulls_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls{/number}","milestones_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/milestones{/number}","notifications_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/labels{/name}","releases_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/releases{/id}","deployments_url":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/deployments","created_at":"2016-12-26T17:27:30Z","updated_at":"2017-07-24T08:46:19Z","pushed_at":"2017-07-24T10:56:50Z","git_url":"git://github.com/ministryofjustice/analytics-platform-ops.git","ssh_url":"git@github.com:ministryofjustice/analytics-platform-ops.git","clone_url":"https://github.com/ministryofjustice/analytics-platform-ops.git","svn_url":"https://github.com/ministryofjustice/analytics-platform-ops","homepage":"","size":1610,"stargazers_count":6,"watchers_count":6,"language":"HCL","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":5,"forks":1,"open_issues":5,"watchers":6,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/89"},"html":{"href":"https://github.com/ministryofjustice/analytics-platform-ops/pull/89"},"issue":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues/89"},"comments":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/issues/89/comments"},"review_comments":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/89/comments"},"review_comment":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/pulls/89/commits"},"statuses":{"href":"https://api.github.com/repos/ministryofjustice/analytics-platform-ops/statuses/8ac4cafcfefc1bd656de2d49760628c7c5a3b13c"}}}}
{"actor":{"display_login":"andyhd"}}