{"action":"created","comment":{"url":"https://api.github.com/repos/mozilla/balrog/pulls/comments/91650474","pull_request_review_id":12161081,"id":91650474,"diff_hunk":"@@ -1880,14 +1890,15 @@ def hasPermission(self, username, thing, action, product=None, transaction=None)\n         except ValueError:\n             return False\n \n-        # If a user has a permission that doesn't explicitly limit the type of\n-        # actions they can perform, they are allowed to do any type of action.\n-        if options.get(\"actions\") and action not in options[\"actions\"]:\n-            return False\n-        # Similarly, permissions without products specified grant that\n-        # that permission without any limitation on the product.\n-        if options.get(\"products\") and product not in options[\"products\"]:\n-            return False\n+        if options:\n+            # If a user has a permission that doesn't explicitly limit the type of\n+            # actions they can perform, they are allowed to do any type of action.\n+            if options.get(\"actions\") and action not in options[\"actions\"]:\n+                return False\n+            # Similarly, permissions without products specified grant that\n+            # that permission without any limitation on the product.\n+            if options.get(\"products\") and product not in options[\"products\"]:\n+                return False\n \n         return True","path":"auslib/db.py","position":206,"original_position":188,"commit_id":"e32cc2f9259e1b13610cfa73e625f218e056a874","original_commit_id":"d191feaed88ae2d71e6a7fa48c356b6ffc05ad5b","user":{"login":"nthomas-mozilla","id":942272,"avatar_url":"https://avatars.githubusercontent.com/u/942272?v=3","gravatar_id":"","url":"https://api.github.com/users/nthomas-mozilla","html_url":"https://github.com/nthomas-mozilla","followers_url":"https://api.github.com/users/nthomas-mozilla/followers","following_url":"https://api.github.com/users/nthomas-mozilla/following{/other_user}","gists_url":"https://api.github.com/users/nthomas-mozilla/gists{/gist_id}","starred_url":"https://api.github.com/users/nthomas-mozilla/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nthomas-mozilla/subscriptions","organizations_url":"https://api.github.com/users/nthomas-mozilla/orgs","repos_url":"https://api.github.com/users/nthomas-mozilla/repos","events_url":"https://api.github.com/users/nthomas-mozilla/events{/privacy}","received_events_url":"https://api.github.com/users/nthomas-mozilla/received_events","type":"User","site_admin":false},"body":"I find _isFieldAbsentOrExplicitlyWhitelisted() a little hard to parse, but that's OK if we test/doc it like mad. Otherwise looks good.","created_at":"2016-12-09T02:38:15Z","updated_at":"2016-12-09T02:38:15Z","html_url":"https://github.com/mozilla/balrog/pull/184#discussion_r91650474","pull_request_url":"https://api.github.com/repos/mozilla/balrog/pulls/184","_links":{"self":{"href":"https://api.github.com/repos/mozilla/balrog/pulls/comments/91650474"},"html":{"href":"https://github.com/mozilla/balrog/pull/184#discussion_r91650474"},"pull_request":{"href":"https://api.github.com/repos/mozilla/balrog/pulls/184"}}},"pull_request":{"url":"https://api.github.com/repos/mozilla/balrog/pulls/184","id":96127465,"html_url":"https://github.com/mozilla/balrog/pull/184","diff_url":"https://github.com/mozilla/balrog/pull/184.diff","patch_url":"https://github.com/mozilla/balrog/pull/184.patch","issue_url":"https://api.github.com/repos/mozilla/balrog/issues/184","number":184,"state":"open","locked":false,"title":"Use custom column types for JSON columns","user":{"login":"mozbhearsum","id":49649,"avatar_url":"https://avatars.githubusercontent.com/u/49649?v=3","gravatar_id":"","url":"https://api.github.com/users/mozbhearsum","html_url":"https://github.com/mozbhearsum","followers_url":"https://api.github.com/users/mozbhearsum/followers","following_url":"https://api.github.com/users/mozbhearsum/following{/other_user}","gists_url":"https://api.github.com/users/mozbhearsum/gists{/gist_id}","starred_url":"https://api.github.com/users/mozbhearsum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozbhearsum/subscriptions","organizations_url":"https://api.github.com/users/mozbhearsum/orgs","repos_url":"https://api.github.com/users/mozbhearsum/repos","events_url":"https://api.github.com/users/mozbhearsum/events{/privacy}","received_events_url":"https://api.github.com/users/mozbhearsum/received_events","type":"User","site_admin":false},"body":"While working on https://bugzilla.mozilla.org/show_bug.cgi?id=1310188 I discovered a bunch of test failures rooted in the releases.data and permissions.options columns. The problem boiled down to the fact that we retrieve the Scheduled Changes from the database when doing an update() or delete(), then pass the data to the base class to do a dry run of enacting the change. Because the rows are retrieved with ScheduledChanges.select(), the mirrored versions of releases.data and permissions.options end up as a string, _not_ a Blob/dict. This causes problems when the base classes try to validate the data, eg when .validate() is called here: https://github.com/mozilla/balrog/blob/a72bbc2289761ec20478665e16d21682a169d46e/auslib/db.py#L1558\r\n\r\nThis pull request fixes the issue by doing serialization/deserialization at the lowest possible level, using a custom column type as described here: http://docs.sqlalchemy.org/en/latest/core/custom_types.html. This might seem a bit heavy handed, but the only alternative I could come up with was monkey-patching self.scheduled_changes.select in Releases/Permissions.__init__, which seemed much worse.\r\n\r\nMost of the patch is simply getting rid of now-unnecessary json.loads/dumps, and fixing some tests to compare against deserialized versions blobs/options instead of strings. A couple of other things of note:\r\n- I had to get a bit more familiar with auslib/admin/views/history.py to make it cope properly. I ended up improving the release diffs to show data version + get rid of of unnecessary context at the same time.\r\n- I made a small interface change to Permissions.getOptions() - it now returns None instead of {} if no options are found to simplify that code a bit. Its only consumer is hasPermission, which has been updated to cope.\r\n- I was initially concerned that I might break caching, or make it less efficient. Manual tests + the cache tests (which verify hits/misses) have given me confidence that I didn't. AFAICT, the only thing that has changed is where deserialization happens, not whether it happens at all.\r\n- I haven't touched the migration scripts because the underlying column type is still the same - the only change is in Python code that sits on top of it. It's a bit weird to have the types differ, but I didn't think it was useful to make a migration script that did nothing.","created_at":"2016-12-01T16:54:04Z","updated_at":"2016-12-09T02:38:15Z","closed_at":null,"merged_at":null,"merge_commit_sha":"70e33a10eb4addac5a0037819456dcfbc81ecd4b","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/mozilla/balrog/pulls/184/commits","review_comments_url":"https://api.github.com/repos/mozilla/balrog/pulls/184/comments","review_comment_url":"https://api.github.com/repos/mozilla/balrog/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mozilla/balrog/issues/184/comments","statuses_url":"https://api.github.com/repos/mozilla/balrog/statuses/e32cc2f9259e1b13610cfa73e625f218e056a874","head":{"label":"mozbhearsum:better-json-columns","ref":"better-json-columns","sha":"e32cc2f9259e1b13610cfa73e625f218e056a874","user":{"login":"mozbhearsum","id":49649,"avatar_url":"https://avatars.githubusercontent.com/u/49649?v=3","gravatar_id":"","url":"https://api.github.com/users/mozbhearsum","html_url":"https://github.com/mozbhearsum","followers_url":"https://api.github.com/users/mozbhearsum/followers","following_url":"https://api.github.com/users/mozbhearsum/following{/other_user}","gists_url":"https://api.github.com/users/mozbhearsum/gists{/gist_id}","starred_url":"https://api.github.com/users/mozbhearsum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozbhearsum/subscriptions","organizations_url":"https://api.github.com/users/mozbhearsum/orgs","repos_url":"https://api.github.com/users/mozbhearsum/repos","events_url":"https://api.github.com/users/mozbhearsum/events{/privacy}","received_events_url":"https://api.github.com/users/mozbhearsum/received_events","type":"User","site_admin":false},"repo":{"id":2570483,"name":"balrog","full_name":"mozbhearsum/balrog","owner":{"login":"mozbhearsum","id":49649,"avatar_url":"https://avatars.githubusercontent.com/u/49649?v=3","gravatar_id":"","url":"https://api.github.com/users/mozbhearsum","html_url":"https://github.com/mozbhearsum","followers_url":"https://api.github.com/users/mozbhearsum/followers","following_url":"https://api.github.com/users/mozbhearsum/following{/other_user}","gists_url":"https://api.github.com/users/mozbhearsum/gists{/gist_id}","starred_url":"https://api.github.com/users/mozbhearsum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozbhearsum/subscriptions","organizations_url":"https://api.github.com/users/mozbhearsum/orgs","repos_url":"https://api.github.com/users/mozbhearsum/repos","events_url":"https://api.github.com/users/mozbhearsum/events{/privacy}","received_events_url":"https://api.github.com/users/mozbhearsum/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/mozbhearsum/balrog","description":"Next generation update service for Mozilla products","fork":true,"url":"https://api.github.com/repos/mozbhearsum/balrog","forks_url":"https://api.github.com/repos/mozbhearsum/balrog/forks","keys_url":"https://api.github.com/repos/mozbhearsum/balrog/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mozbhearsum/balrog/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mozbhearsum/balrog/teams","hooks_url":"https://api.github.com/repos/mozbhearsum/balrog/hooks","issue_events_url":"https://api.github.com/repos/mozbhearsum/balrog/issues/events{/number}","events_url":"https://api.github.com/repos/mozbhearsum/balrog/events","assignees_url":"https://api.github.com/repos/mozbhearsum/balrog/assignees{/user}","branches_url":"https://api.github.com/repos/mozbhearsum/balrog/branches{/branch}","tags_url":"https://api.github.com/repos/mozbhearsum/balrog/tags","blobs_url":"https://api.github.com/repos/mozbhearsum/balrog/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mozbhearsum/balrog/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mozbhearsum/balrog/git/refs{/sha}","trees_url":"https://api.github.com/repos/mozbhearsum/balrog/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mozbhearsum/balrog/statuses/{sha}","languages_url":"https://api.github.com/repos/mozbhearsum/balrog/languages","stargazers_url":"https://api.github.com/repos/mozbhearsum/balrog/stargazers","contributors_url":"https://api.github.com/repos/mozbhearsum/balrog/contributors","subscribers_url":"https://api.github.com/repos/mozbhearsum/balrog/subscribers","subscription_url":"https://api.github.com/repos/mozbhearsum/balrog/subscription","commits_url":"https://api.github.com/repos/mozbhearsum/balrog/commits{/sha}","git_commits_url":"https://api.github.com/repos/mozbhearsum/balrog/git/commits{/sha}","comments_url":"https://api.github.com/repos/mozbhearsum/balrog/comments{/number}","issue_comment_url":"https://api.github.com/repos/mozbhearsum/balrog/issues/comments{/number}","contents_url":"https://api.github.com/repos/mozbhearsum/balrog/contents/{+path}","compare_url":"https://api.github.com/repos/mozbhearsum/balrog/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mozbhearsum/balrog/merges","archive_url":"https://api.github.com/repos/mozbhearsum/balrog/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mozbhearsum/balrog/downloads","issues_url":"https://api.github.com/repos/mozbhearsum/balrog/issues{/number}","pulls_url":"https://api.github.com/repos/mozbhearsum/balrog/pulls{/number}","milestones_url":"https://api.github.com/repos/mozbhearsum/balrog/milestones{/number}","notifications_url":"https://api.github.com/repos/mozbhearsum/balrog/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mozbhearsum/balrog/labels{/name}","releases_url":"https://api.github.com/repos/mozbhearsum/balrog/releases{/id}","deployments_url":"https://api.github.com/repos/mozbhearsum/balrog/deployments","created_at":"2011-10-13T15:58:54Z","updated_at":"2016-08-31T17:23:24Z","pushed_at":"2016-12-02T21:11:29Z","git_url":"git://github.com/mozbhearsum/balrog.git","ssh_url":"git@github.com:mozbhearsum/balrog.git","clone_url":"https://github.com/mozbhearsum/balrog.git","svn_url":"https://github.com/mozbhearsum/balrog","homepage":"","size":63791,"stargazers_count":2,"watchers_count":2,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":2,"default_branch":"master"}},"base":{"label":"mozilla:master","ref":"master","sha":"e9a7d5f902ab8337a57ee65ac58ca55dca325d95","user":{"login":"mozilla","id":131524,"avatar_url":"https://avatars.githubusercontent.com/u/131524?v=3","gravatar_id":"","url":"https://api.github.com/users/mozilla","html_url":"https://github.com/mozilla","followers_url":"https://api.github.com/users/mozilla/followers","following_url":"https://api.github.com/users/mozilla/following{/other_user}","gists_url":"https://api.github.com/users/mozilla/gists{/gist_id}","starred_url":"https://api.github.com/users/mozilla/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozilla/subscriptions","organizations_url":"https://api.github.com/users/mozilla/orgs","repos_url":"https://api.github.com/users/mozilla/repos","events_url":"https://api.github.com/users/mozilla/events{/privacy}","received_events_url":"https://api.github.com/users/mozilla/received_events","type":"Organization","site_admin":false},"repo":{"id":2570444,"name":"balrog","full_name":"mozilla/balrog","owner":{"login":"mozilla","id":131524,"avatar_url":"https://avatars.githubusercontent.com/u/131524?v=3","gravatar_id":"","url":"https://api.github.com/users/mozilla","html_url":"https://github.com/mozilla","followers_url":"https://api.github.com/users/mozilla/followers","following_url":"https://api.github.com/users/mozilla/following{/other_user}","gists_url":"https://api.github.com/users/mozilla/gists{/gist_id}","starred_url":"https://api.github.com/users/mozilla/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozilla/subscriptions","organizations_url":"https://api.github.com/users/mozilla/orgs","repos_url":"https://api.github.com/users/mozilla/repos","events_url":"https://api.github.com/users/mozilla/events{/privacy}","received_events_url":"https://api.github.com/users/mozilla/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mozilla/balrog","description":"Next generation update service for Mozilla products","fork":false,"url":"https://api.github.com/repos/mozilla/balrog","forks_url":"https://api.github.com/repos/mozilla/balrog/forks","keys_url":"https://api.github.com/repos/mozilla/balrog/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mozilla/balrog/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mozilla/balrog/teams","hooks_url":"https://api.github.com/repos/mozilla/balrog/hooks","issue_events_url":"https://api.github.com/repos/mozilla/balrog/issues/events{/number}","events_url":"https://api.github.com/repos/mozilla/balrog/events","assignees_url":"https://api.github.com/repos/mozilla/balrog/assignees{/user}","branches_url":"https://api.github.com/repos/mozilla/balrog/branches{/branch}","tags_url":"https://api.github.com/repos/mozilla/balrog/tags","blobs_url":"https://api.github.com/repos/mozilla/balrog/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mozilla/balrog/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mozilla/balrog/git/refs{/sha}","trees_url":"https://api.github.com/repos/mozilla/balrog/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mozilla/balrog/statuses/{sha}","languages_url":"https://api.github.com/repos/mozilla/balrog/languages","stargazers_url":"https://api.github.com/repos/mozilla/balrog/stargazers","contributors_url":"https://api.github.com/repos/mozilla/balrog/contributors","subscribers_url":"https://api.github.com/repos/mozilla/balrog/subscribers","subscription_url":"https://api.github.com/repos/mozilla/balrog/subscription","commits_url":"https://api.github.com/repos/mozilla/balrog/commits{/sha}","git_commits_url":"https://api.github.com/repos/mozilla/balrog/git/commits{/sha}","comments_url":"https://api.github.com/repos/mozilla/balrog/comments{/number}","issue_comment_url":"https://api.github.com/repos/mozilla/balrog/issues/comments{/number}","contents_url":"https://api.github.com/repos/mozilla/balrog/contents/{+path}","compare_url":"https://api.github.com/repos/mozilla/balrog/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mozilla/balrog/merges","archive_url":"https://api.github.com/repos/mozilla/balrog/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mozilla/balrog/downloads","issues_url":"https://api.github.com/repos/mozilla/balrog/issues{/number}","pulls_url":"https://api.github.com/repos/mozilla/balrog/pulls{/number}","milestones_url":"https://api.github.com/repos/mozilla/balrog/milestones{/number}","notifications_url":"https://api.github.com/repos/mozilla/balrog/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mozilla/balrog/labels{/name}","releases_url":"https://api.github.com/repos/mozilla/balrog/releases{/id}","deployments_url":"https://api.github.com/repos/mozilla/balrog/deployments","created_at":"2011-10-13T15:51:47Z","updated_at":"2016-12-06T04:18:06Z","pushed_at":"2016-12-06T18:55:12Z","git_url":"git://github.com/mozilla/balrog.git","ssh_url":"git@github.com:mozilla/balrog.git","clone_url":"https://github.com/mozilla/balrog.git","svn_url":"https://github.com/mozilla/balrog","homepage":"","size":62177,"stargazers_count":27,"watchers_count":27,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":50,"mirror_url":null,"open_issues_count":5,"forks":50,"open_issues":5,"watchers":27,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mozilla/balrog/pulls/184"},"html":{"href":"https://github.com/mozilla/balrog/pull/184"},"issue":{"href":"https://api.github.com/repos/mozilla/balrog/issues/184"},"comments":{"href":"https://api.github.com/repos/mozilla/balrog/issues/184/comments"},"review_comments":{"href":"https://api.github.com/repos/mozilla/balrog/pulls/184/comments"},"review_comment":{"href":"https://api.github.com/repos/mozilla/balrog/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mozilla/balrog/pulls/184/commits"},"statuses":{"href":"https://api.github.com/repos/mozilla/balrog/statuses/e32cc2f9259e1b13610cfa73e625f218e056a874"}}}}
{"actor":{"display_login":"nthomas-mozilla"}}