GitHub PullRequestReviewCommentEvent at 2014-11-12 01:28:23
Repository: puppet
Actor: hlindberg
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments/20195873","id":20195873,"diff_hunk":"@@ -56,13 +56,34 @@ def call(request, response)\n   end\n \n   def uri2indirection(http_method, uri, params)\n-    environment, indirection, key = uri.split(\"/\", 4)[1..-1] # the first field is always nil because of the leading slash\n+    indirection, key = uri.split(\"/\", 3)[1..-1] # the first field is always nil because of the leading slash\n \n-    raise ArgumentError, \"The environment must be purely alphanumeric, not '#{environment}'\" unless Puppet::Node::Environment.valid_name?(environment)\n     raise ArgumentError, \"The indirection name must be purely alphanumeric, not '#{indirection}'\" unless indirection =~ /^\\w+$/\n \n     method = indirection_method(http_method, indirection)\n \n+    if method == :save\n+      # In the case of a PUT request which maps to a `save` indirection, the HTTP\n+      # specification doesn't allow a query string, so we can't put the environment\n+      # there.  It would need to go in the request body.  However, since the\n+      # indirector hides the deserialization of the body behind the 'model' object\n+      # for each different indirection, we don't have access to the body yet either.\n+      #\n+      # After discussion, it sounds like the only two 'save' indirections that come\n+      # through this HTTP layer are 'report' and 'file_bucket', and looking at the\n+      # implementations for those, they don't reference the environment from the\n+      # indirector request at all, so it seems safe to just set it to 'production'\n+      # for those types of requests.  We should find a way to not have to\n+      # special-case this in the long run (e.g. maybe just getting rid of the 'save'\n+      # functionality in the HTTP layer of the indirector?  Replacing it with new\n+      # endpoints that explicitly handle PUT requests?)\n+      environment = \"production\"\n+    else\n+      environment = params.delete(:environment)\n+    end","path":"lib/puppet/network/http/api/v1.rb","position":30,"original_position":30,"commit_id":"a344a989ad0a8bfe35aa63b50ea6aec0217b0a73","original_commit_id":"a344a989ad0a8bfe35aa63b50ea6aec0217b0a73","user":{"login":"hlindberg","id":563066,"avatar_url":"https://avatars.githubusercontent.com/u/563066?v=3","gravatar_id":"","url":"https://api.github.com/users/hlindberg","html_url":"https://github.com/hlindberg","followers_url":"https://api.github.com/users/hlindberg/followers","following_url":"https://api.github.com/users/hlindberg/following{/other_user}","gists_url":"https://api.github.com/users/hlindberg/gists{/gist_id}","starred_url":"https://api.github.com/users/hlindberg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hlindberg/subscriptions","organizations_url":"https://api.github.com/users/hlindberg/orgs","repos_url":"https://api.github.com/users/hlindberg/repos","events_url":"https://api.github.com/users/hlindberg/events{/privacy}","received_events_url":"https://api.github.com/users/hlindberg/received_events","type":"User","site_admin":false},"body":"I am concerned about using \"production\" for this. Is the actual environment encoded inside of the report data? IIRC there is the notion of report processors, and I do not know exactly how they work, if they can be different per environment (i.e. delivered in modules) and that the environment has to be correct, or they are a system level thing.","created_at":"2014-11-12T01:28:23Z","updated_at":"2014-11-12T01:28:23Z","html_url":"https://github.com/puppetlabs/puppet/pull/3297#discussion_r20195873","pull_request_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/3297","_links":{"self":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments/20195873"},"html":{"href":"https://github.com/puppetlabs/puppet/pull/3297#discussion_r20195873"},"pull_request":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/3297"}}},"pull_request":{"url":"https://api.github.com/repos/puppetlabs/puppet/pulls/3297","id":24186779,"html_url":"https://github.com/puppetlabs/puppet/pull/3297","diff_url":"https://github.com/puppetlabs/puppet/pull/3297.diff","patch_url":"https://github.com/puppetlabs/puppet/pull/3297.patch","issue_url":"https://api.github.com/repos/puppetlabs/puppet/issues/3297","number":3297,"state":"open","locked":false,"title":"FOR REVIEW: (PUP-3642) Remove environment name from URL path","user":{"login":"cprice404","id":1341071,"avatar_url":"https://avatars.githubusercontent.com/u/1341071?v=3","gravatar_id":"","url":"https://api.github.com/users/cprice404","html_url":"https://github.com/cprice404","followers_url":"https://api.github.com/users/cprice404/followers","following_url":"https://api.github.com/users/cprice404/following{/other_user}","gists_url":"https://api.github.com/users/cprice404/gists{/gist_id}","starred_url":"https://api.github.com/users/cprice404/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cprice404/subscriptions","organizations_url":"https://api.github.com/users/cprice404/orgs","repos_url":"https://api.github.com/users/cprice404/repos","events_url":"https://api.github.com/users/cprice404/events{/privacy}","received_events_url":"https://api.github.com/users/cprice404/received_events","type":"User","site_admin":false},"body":"FOR REVIEW ONLY\r\n----------------\r\nThis probably isn't quite ready yet.  I need to talk to @hlindberg , @kylog , or @joshcooper and make sure that what I'm doing here for `save` requests is sane; I might need to go put in a bit more string massaging to prevent putting extra '?' or '&' characters into the URL when they're not needed; and I need to make sure that @ahpook is ready to approve this change for 4.0.\r\n\r\n\r\nThis commit makes some changes to the indirector network layer\r\nsuch that the environment name is no longer a part of the main\r\nURL path structure.  Instead, the environment becomes a query\r\nparameter for GET requests, or part of the form body for POST\r\nrequests.  This will allow us to have a more finite URL space\r\nand run Puppet in the same webserver with other apps, without\r\nworrying about URL collisions.","created_at":"2014-11-10T23:30:05Z","updated_at":"2014-11-12T01:28:23Z","closed_at":null,"merged_at":null,"merge_commit_sha":"914a9f9d29c5c52b7ac76b0891c31b857d6d1f2b","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/3297/commits","review_comments_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/3297/comments","review_comment_url":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments/{number}","comments_url":"https://api.github.com/repos/puppetlabs/puppet/issues/3297/comments","statuses_url":"https://api.github.com/repos/puppetlabs/puppet/statuses/a344a989ad0a8bfe35aa63b50ea6aec0217b0a73","head":{"label":"cprice404:feature/master/PUP-3642-remove-env-from-url-path","ref":"feature/master/PUP-3642-remove-env-from-url-path","sha":"a344a989ad0a8bfe35aa63b50ea6aec0217b0a73","user":{"login":"cprice404","id":1341071,"avatar_url":"https://avatars.githubusercontent.com/u/1341071?v=3","gravatar_id":"","url":"https://api.github.com/users/cprice404","html_url":"https://github.com/cprice404","followers_url":"https://api.github.com/users/cprice404/followers","following_url":"https://api.github.com/users/cprice404/following{/other_user}","gists_url":"https://api.github.com/users/cprice404/gists{/gist_id}","starred_url":"https://api.github.com/users/cprice404/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cprice404/subscriptions","organizations_url":"https://api.github.com/users/cprice404/orgs","repos_url":"https://api.github.com/users/cprice404/repos","events_url":"https://api.github.com/users/cprice404/events{/privacy}","received_events_url":"https://api.github.com/users/cprice404/received_events","type":"User","site_admin":false},"repo":{"id":3204103,"name":"puppet","full_name":"cprice404/puppet","owner":{"login":"cprice404","id":1341071,"avatar_url":"https://avatars.githubusercontent.com/u/1341071?v=3","gravatar_id":"","url":"https://api.github.com/users/cprice404","html_url":"https://github.com/cprice404","followers_url":"https://api.github.com/users/cprice404/followers","following_url":"https://api.github.com/users/cprice404/following{/other_user}","gists_url":"https://api.github.com/users/cprice404/gists{/gist_id}","starred_url":"https://api.github.com/users/cprice404/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cprice404/subscriptions","organizations_url":"https://api.github.com/users/cprice404/orgs","repos_url":"https://api.github.com/users/cprice404/repos","events_url":"https://api.github.com/users/cprice404/events{/privacy}","received_events_url":"https://api.github.com/users/cprice404/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/cprice404/puppet","description":"Server automation framework and application","fork":true,"url":"https://api.github.com/repos/cprice404/puppet","forks_url":"https://api.github.com/repos/cprice404/puppet/forks","keys_url":"https://api.github.com/repos/cprice404/puppet/keys{/key_id}","collaborators_url":"https://api.github.com/repos/cprice404/puppet/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/cprice404/puppet/teams","hooks_url":"https://api.github.com/repos/cprice404/puppet/hooks","issue_events_url":"https://api.github.com/repos/cprice404/puppet/issues/events{/number}","events_url":"https://api.github.com/repos/cprice404/puppet/events","assignees_url":"https://api.github.com/repos/cprice404/puppet/assignees{/user}","branches_url":"https://api.github.com/repos/cprice404/puppet/branches{/branch}","tags_url":"https://api.github.com/repos/cprice404/puppet/tags","blobs_url":"https://api.github.com/repos/cprice404/puppet/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/cprice404/puppet/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/cprice404/puppet/git/refs{/sha}","trees_url":"https://api.github.com/repos/cprice404/puppet/git/trees{/sha}","statuses_url":"https://api.github.com/repos/cprice404/puppet/statuses/{sha}","languages_url":"https://api.github.com/repos/cprice404/puppet/languages","stargazers_url":"https://api.github.com/repos/cprice404/puppet/stargazers","contributors_url":"https://api.github.com/repos/cprice404/puppet/contributors","subscribers_url":"https://api.github.com/repos/cprice404/puppet/subscribers","subscription_url":"https://api.github.com/repos/cprice404/puppet/subscription","commits_url":"https://api.github.com/repos/cprice404/puppet/commits{/sha}","git_commits_url":"https://api.github.com/repos/cprice404/puppet/git/commits{/sha}","comments_url":"https://api.github.com/repos/cprice404/puppet/comments{/number}","issue_comment_url":"https://api.github.com/repos/cprice404/puppet/issues/comments/{number}","contents_url":"https://api.github.com/repos/cprice404/puppet/contents/{+path}","compare_url":"https://api.github.com/repos/cprice404/puppet/compare/{base}...{head}","merges_url":"https://api.github.com/repos/cprice404/puppet/merges","archive_url":"https://api.github.com/repos/cprice404/puppet/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/cprice404/puppet/downloads","issues_url":"https://api.github.com/repos/cprice404/puppet/issues{/number}","pulls_url":"https://api.github.com/repos/cprice404/puppet/pulls{/number}","milestones_url":"https://api.github.com/repos/cprice404/puppet/milestones{/number}","notifications_url":"https://api.github.com/repos/cprice404/puppet/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/cprice404/puppet/labels{/name}","releases_url":"https://api.github.com/repos/cprice404/puppet/releases{/id}","created_at":"2012-01-17T22:55:56Z","updated_at":"2014-06-24T04:10:45Z","pushed_at":"2014-11-11T00:23:58Z","git_url":"git://github.com/cprice404/puppet.git","ssh_url":"git@github.com:cprice404/puppet.git","clone_url":"https://github.com/cprice404/puppet.git","svn_url":"https://github.com/cprice404/puppet","homepage":"http://www.puppetlabs.com/puppet/introduction/","size":43926,"stargazers_count":1,"watchers_count":1,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"puppetlabs:master","ref":"master","sha":"5ec168479493a17ef426bbe5d19575943a1078ec","user":{"login":"puppetlabs","id":234268,"avatar_url":"https://avatars.githubusercontent.com/u/234268?v=3","gravatar_id":"","url":"https://api.github.com/users/puppetlabs","html_url":"https://github.com/puppetlabs","followers_url":"https://api.github.com/users/puppetlabs/followers","following_url":"https://api.github.com/users/puppetlabs/following{/other_user}","gists_url":"https://api.github.com/users/puppetlabs/gists{/gist_id}","starred_url":"https://api.github.com/users/puppetlabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/puppetlabs/subscriptions","organizations_url":"https://api.github.com/users/puppetlabs/orgs","repos_url":"https://api.github.com/users/puppetlabs/repos","events_url":"https://api.github.com/users/puppetlabs/events{/privacy}","received_events_url":"https://api.github.com/users/puppetlabs/received_events","type":"Organization","site_admin":false},"repo":{"id":910744,"name":"puppet","full_name":"puppetlabs/puppet","owner":{"login":"puppetlabs","id":234268,"avatar_url":"https://avatars.githubusercontent.com/u/234268?v=3","gravatar_id":"","url":"https://api.github.com/users/puppetlabs","html_url":"https://github.com/puppetlabs","followers_url":"https://api.github.com/users/puppetlabs/followers","following_url":"https://api.github.com/users/puppetlabs/following{/other_user}","gists_url":"https://api.github.com/users/puppetlabs/gists{/gist_id}","starred_url":"https://api.github.com/users/puppetlabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/puppetlabs/subscriptions","organizations_url":"https://api.github.com/users/puppetlabs/orgs","repos_url":"https://api.github.com/users/puppetlabs/repos","events_url":"https://api.github.com/users/puppetlabs/events{/privacy}","received_events_url":"https://api.github.com/users/puppetlabs/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/puppetlabs/puppet","description":"Server automation framework and application","fork":false,"url":"https://api.github.com/repos/puppetlabs/puppet","forks_url":"https://api.github.com/repos/puppetlabs/puppet/forks","keys_url":"https://api.github.com/repos/puppetlabs/puppet/keys{/key_id}","collaborators_url":"https://api.github.com/repos/puppetlabs/puppet/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/puppetlabs/puppet/teams","hooks_url":"https://api.github.com/repos/puppetlabs/puppet/hooks","issue_events_url":"https://api.github.com/repos/puppetlabs/puppet/issues/events{/number}","events_url":"https://api.github.com/repos/puppetlabs/puppet/events","assignees_url":"https://api.github.com/repos/puppetlabs/puppet/assignees{/user}","branches_url":"https://api.github.com/repos/puppetlabs/puppet/branches{/branch}","tags_url":"https://api.github.com/repos/puppetlabs/puppet/tags","blobs_url":"https://api.github.com/repos/puppetlabs/puppet/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/puppetlabs/puppet/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/puppetlabs/puppet/git/refs{/sha}","trees_url":"https://api.github.com/repos/puppetlabs/puppet/git/trees{/sha}","statuses_url":"https://api.github.com/repos/puppetlabs/puppet/statuses/{sha}","languages_url":"https://api.github.com/repos/puppetlabs/puppet/languages","stargazers_url":"https://api.github.com/repos/puppetlabs/puppet/stargazers","contributors_url":"https://api.github.com/repos/puppetlabs/puppet/contributors","subscribers_url":"https://api.github.com/repos/puppetlabs/puppet/subscribers","subscription_url":"https://api.github.com/repos/puppetlabs/puppet/subscription","commits_url":"https://api.github.com/repos/puppetlabs/puppet/commits{/sha}","git_commits_url":"https://api.github.com/repos/puppetlabs/puppet/git/commits{/sha}","comments_url":"https://api.github.com/repos/puppetlabs/puppet/comments{/number}","issue_comment_url":"https://api.github.com/repos/puppetlabs/puppet/issues/comments/{number}","contents_url":"https://api.github.com/repos/puppetlabs/puppet/contents/{+path}","compare_url":"https://api.github.com/repos/puppetlabs/puppet/compare/{base}...{head}","merges_url":"https://api.github.com/repos/puppetlabs/puppet/merges","archive_url":"https://api.github.com/repos/puppetlabs/puppet/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/puppetlabs/puppet/downloads","issues_url":"https://api.github.com/repos/puppetlabs/puppet/issues{/number}","pulls_url":"https://api.github.com/repos/puppetlabs/puppet/pulls{/number}","milestones_url":"https://api.github.com/repos/puppetlabs/puppet/milestones{/number}","notifications_url":"https://api.github.com/repos/puppetlabs/puppet/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/puppetlabs/puppet/labels{/name}","releases_url":"https://api.github.com/repos/puppetlabs/puppet/releases{/id}","created_at":"2010-09-14T19:26:44Z","updated_at":"2014-11-11T19:50:51Z","pushed_at":"2014-11-11T23:45:05Z","git_url":"git://github.com/puppetlabs/puppet.git","ssh_url":"git@github.com:puppetlabs/puppet.git","clone_url":"https://github.com/puppetlabs/puppet.git","svn_url":"https://github.com/puppetlabs/puppet","homepage":"http://www.puppetlabs.com/puppet/introduction/","size":115217,"stargazers_count":2711,"watchers_count":2711,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1192,"mirror_url":null,"open_issues_count":48,"forks":1192,"open_issues":48,"watchers":2711,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/3297"},"html":{"href":"https://github.com/puppetlabs/puppet/pull/3297"},"issue":{"href":"https://api.github.com/repos/puppetlabs/puppet/issues/3297"},"comments":{"href":"https://api.github.com/repos/puppetlabs/puppet/issues/3297/comments"},"review_comments":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/3297/comments"},"review_comment":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/puppetlabs/puppet/pulls/3297/commits"},"statuses":{"href":"https://api.github.com/repos/puppetlabs/puppet/statuses/a344a989ad0a8bfe35aa63b50ea6aec0217b0a73"}}}}