GitHub PullRequestReviewCommentEvent at 2014-11-11 14:09:28
Repository: manageiq
Actor: blomquisg
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/ManageIQ/manageiq/pulls/comments/20151102","id":20151102,"diff_hunk":"@@ -0,0 +1,5 @@\n+class AddProviderComponentToEmsConnections < ActiveRecord::Migration\n+  def change\n+    add_column :ems_connections, :provider_component, :string","path":"vmdb/db/migrate/20141110212624_add_provider_component_to_ems_connections.rb","position":3,"original_position":3,"commit_id":"edc3f7e06057ac626de2eabb62809f4894a8e9ff","original_commit_id":"edc3f7e06057ac626de2eabb62809f4894a8e9ff","user":{"login":"blomquisg","id":14183,"avatar_url":"https://avatars.githubusercontent.com/u/14183?v=3","gravatar_id":"","url":"https://api.github.com/users/blomquisg","html_url":"https://github.com/blomquisg","followers_url":"https://api.github.com/users/blomquisg/followers","following_url":"https://api.github.com/users/blomquisg/following{/other_user}","gists_url":"https://api.github.com/users/blomquisg/gists{/gist_id}","starred_url":"https://api.github.com/users/blomquisg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/blomquisg/subscriptions","organizations_url":"https://api.github.com/users/blomquisg/orgs","repos_url":"https://api.github.com/users/blomquisg/repos","events_url":"https://api.github.com/users/blomquisg/events{/privacy}","received_events_url":"https://api.github.com/users/blomquisg/received_events","type":"User","site_admin":false},"body":"Oh, almost forgot.\r\n\r\nRemember that each `ems_connection` may have to satisfy more than one `provider_component`.  E.g.:\r\n\r\n* RHEV\r\n    * Default credentials handles both `refresh` and `events`.\r\n    * Metrics credentials handles `metrics`.\r\n* OpenStack\r\n    * Default credentials handles both `refresh` and `metrics`.\r\n    * AMQP credentials handles `events`.\r\n* Microsoft\r\n    * Default credentials handle `refresh`.\r\n    * Operation Center credentials handles `events` and `metrics`.\r\n\r\nWe actually have a nice representation of the various combinations with these three providers.","created_at":"2014-11-11T14:09:28Z","updated_at":"2014-11-11T14:09:28Z","html_url":"https://github.com/ManageIQ/manageiq/pull/1037#discussion_r20151102","pull_request_url":"https://api.github.com/repos/ManageIQ/manageiq/pulls/1037","_links":{"self":{"href":"https://api.github.com/repos/ManageIQ/manageiq/pulls/comments/20151102"},"html":{"href":"https://github.com/ManageIQ/manageiq/pull/1037#discussion_r20151102"},"pull_request":{"href":"https://api.github.com/repos/ManageIQ/manageiq/pulls/1037"}}},"pull_request":{"url":"https://api.github.com/repos/ManageIQ/manageiq/pulls/1037","id":24174720,"html_url":"https://github.com/ManageIQ/manageiq/pull/1037","diff_url":"https://github.com/ManageIQ/manageiq/pull/1037.diff","patch_url":"https://github.com/ManageIQ/manageiq/pull/1037.patch","issue_url":"https://api.github.com/repos/ManageIQ/manageiq/issues/1037","number":1037,"state":"open","locked":false,"title":"[WIP] In progress -  multiple endpoints per provider feature","user":{"login":"bronaghs","id":4379675,"avatar_url":"https://avatars.githubusercontent.com/u/4379675?v=3","gravatar_id":"","url":"https://api.github.com/users/bronaghs","html_url":"https://github.com/bronaghs","followers_url":"https://api.github.com/users/bronaghs/followers","following_url":"https://api.github.com/users/bronaghs/following{/other_user}","gists_url":"https://api.github.com/users/bronaghs/gists{/gist_id}","starred_url":"https://api.github.com/users/bronaghs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bronaghs/subscriptions","organizations_url":"https://api.github.com/users/bronaghs/orgs","repos_url":"https://api.github.com/users/bronaghs/repos","events_url":"https://api.github.com/users/bronaghs/events{/privacy}","received_events_url":"https://api.github.com/users/bronaghs/received_events","type":"User","site_admin":false},"body":"Below is an overview that I hope will help you understand the code changes that make up this new feature.  Please focus on overall design, keep it high level. Once it is working I will sweep through and make improvements.\r\n\r\nGeneral overview:\r\n\r\nWe now have a class called EMS_Connection. Which kind of “sits between” the ExtManagementSytem and the Authentication classes.\r\nThis PR excludes ALL changes to the UI, but I envision we will have a UI frame for each EMS_Connection encompassing at least: \r\nIP\r\nport\r\nhostname\r\nprovider_component eg “refresh” or “events”\r\nexisting User Credentials box\r\n\r\nEach ExtManagementSystem will have one or more EMS_Connection(s) with the ems_id as the foreign key.\r\nThe EMS_Connection will have one or more Authentications with the ems_connection id as the foreign key.\r\n\r\nThe EMS_Connection includes the AuthenticationMixin making it easy for it to take care of the many methods we already have in place for saving and checking credentials.\r\n\r\nA couple of things:\r\n\r\n1) The EMS still takes care of the validate_credentials and connect methods but now depends on the credentials being passed into each method in the options hash as opposed to being directly accessible from the EMS itself. (remember the validate_credentials only has access to variables stored in memory, not the database.)\r\n\r\n2) Workers\r\n - In order for the workers to start up we check if the EMS has valid credentials configured (per_ems_worker_mixin)\r\n After the workers have been initialized (ems_refresh_core_worker and ems_refresher_worker) we check the credentials of the EMS again, like this:\r\n\r\n@ems.authentication_check\r\n\r\nThe new approach is to get a handle on the EMS_Connection as per the example below:\r\n@ems.ems_connections.find_by_provider_component(\"refresh\").authentication_check\r\n\r\nOnce I have everything working I will see if I can improve on this without passing in the string, maybe create a new method in the ext_management_system class which uses the class name instead, not sure.\r\nI follow this approach in other places too eg the refresher parsers.","created_at":"2014-11-10T20:26:29Z","updated_at":"2014-11-11T14:09:28Z","closed_at":null,"merged_at":null,"merge_commit_sha":null,"assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/ManageIQ/manageiq/pulls/1037/commits","review_comments_url":"https://api.github.com/repos/ManageIQ/manageiq/pulls/1037/comments","review_comment_url":"https://api.github.com/repos/ManageIQ/manageiq/pulls/comments/{number}","comments_url":"https://api.github.com/repos/ManageIQ/manageiq/issues/1037/comments","statuses_url":"https://api.github.com/repos/ManageIQ/manageiq/statuses/edc3f7e06057ac626de2eabb62809f4894a8e9ff","head":{"label":"bronaghs:provider_multiple_connections","ref":"provider_multiple_connections","sha":"edc3f7e06057ac626de2eabb62809f4894a8e9ff","user":{"login":"bronaghs","id":4379675,"avatar_url":"https://avatars.githubusercontent.com/u/4379675?v=3","gravatar_id":"","url":"https://api.github.com/users/bronaghs","html_url":"https://github.com/bronaghs","followers_url":"https://api.github.com/users/bronaghs/followers","following_url":"https://api.github.com/users/bronaghs/following{/other_user}","gists_url":"https://api.github.com/users/bronaghs/gists{/gist_id}","starred_url":"https://api.github.com/users/bronaghs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bronaghs/subscriptions","organizations_url":"https://api.github.com/users/bronaghs/orgs","repos_url":"https://api.github.com/users/bronaghs/repos","events_url":"https://api.github.com/users/bronaghs/events{/privacy}","received_events_url":"https://api.github.com/users/bronaghs/received_events","type":"User","site_admin":false},"repo":{"id":20886201,"name":"manageiq","full_name":"bronaghs/manageiq","owner":{"login":"bronaghs","id":4379675,"avatar_url":"https://avatars.githubusercontent.com/u/4379675?v=3","gravatar_id":"","url":"https://api.github.com/users/bronaghs","html_url":"https://github.com/bronaghs","followers_url":"https://api.github.com/users/bronaghs/followers","following_url":"https://api.github.com/users/bronaghs/following{/other_user}","gists_url":"https://api.github.com/users/bronaghs/gists{/gist_id}","starred_url":"https://api.github.com/users/bronaghs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bronaghs/subscriptions","organizations_url":"https://api.github.com/users/bronaghs/orgs","repos_url":"https://api.github.com/users/bronaghs/repos","events_url":"https://api.github.com/users/bronaghs/events{/privacy}","received_events_url":"https://api.github.com/users/bronaghs/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/bronaghs/manageiq","description":"ManageIQ Cloud Management Platform","fork":true,"url":"https://api.github.com/repos/bronaghs/manageiq","forks_url":"https://api.github.com/repos/bronaghs/manageiq/forks","keys_url":"https://api.github.com/repos/bronaghs/manageiq/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bronaghs/manageiq/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bronaghs/manageiq/teams","hooks_url":"https://api.github.com/repos/bronaghs/manageiq/hooks","issue_events_url":"https://api.github.com/repos/bronaghs/manageiq/issues/events{/number}","events_url":"https://api.github.com/repos/bronaghs/manageiq/events","assignees_url":"https://api.github.com/repos/bronaghs/manageiq/assignees{/user}","branches_url":"https://api.github.com/repos/bronaghs/manageiq/branches{/branch}","tags_url":"https://api.github.com/repos/bronaghs/manageiq/tags","blobs_url":"https://api.github.com/repos/bronaghs/manageiq/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bronaghs/manageiq/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bronaghs/manageiq/git/refs{/sha}","trees_url":"https://api.github.com/repos/bronaghs/manageiq/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bronaghs/manageiq/statuses/{sha}","languages_url":"https://api.github.com/repos/bronaghs/manageiq/languages","stargazers_url":"https://api.github.com/repos/bronaghs/manageiq/stargazers","contributors_url":"https://api.github.com/repos/bronaghs/manageiq/contributors","subscribers_url":"https://api.github.com/repos/bronaghs/manageiq/subscribers","subscription_url":"https://api.github.com/repos/bronaghs/manageiq/subscription","commits_url":"https://api.github.com/repos/bronaghs/manageiq/commits{/sha}","git_commits_url":"https://api.github.com/repos/bronaghs/manageiq/git/commits{/sha}","comments_url":"https://api.github.com/repos/bronaghs/manageiq/comments{/number}","issue_comment_url":"https://api.github.com/repos/bronaghs/manageiq/issues/comments/{number}","contents_url":"https://api.github.com/repos/bronaghs/manageiq/contents/{+path}","compare_url":"https://api.github.com/repos/bronaghs/manageiq/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bronaghs/manageiq/merges","archive_url":"https://api.github.com/repos/bronaghs/manageiq/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bronaghs/manageiq/downloads","issues_url":"https://api.github.com/repos/bronaghs/manageiq/issues{/number}","pulls_url":"https://api.github.com/repos/bronaghs/manageiq/pulls{/number}","milestones_url":"https://api.github.com/repos/bronaghs/manageiq/milestones{/number}","notifications_url":"https://api.github.com/repos/bronaghs/manageiq/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bronaghs/manageiq/labels{/name}","releases_url":"https://api.github.com/repos/bronaghs/manageiq/releases{/id}","created_at":"2014-06-16T13:40:41Z","updated_at":"2014-06-23T15:25:44Z","pushed_at":"2014-11-10T21:28:52Z","git_url":"git://github.com/bronaghs/manageiq.git","ssh_url":"git@github.com:bronaghs/manageiq.git","clone_url":"https://github.com/bronaghs/manageiq.git","svn_url":"https://github.com/bronaghs/manageiq","homepage":null,"size":58032,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"ManageIQ:master","ref":"master","sha":"20b89c1871bbea2a9b30a912e1235785ed3b6a83","user":{"login":"ManageIQ","id":1125575,"avatar_url":"https://avatars.githubusercontent.com/u/1125575?v=3","gravatar_id":"","url":"https://api.github.com/users/ManageIQ","html_url":"https://github.com/ManageIQ","followers_url":"https://api.github.com/users/ManageIQ/followers","following_url":"https://api.github.com/users/ManageIQ/following{/other_user}","gists_url":"https://api.github.com/users/ManageIQ/gists{/gist_id}","starred_url":"https://api.github.com/users/ManageIQ/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ManageIQ/subscriptions","organizations_url":"https://api.github.com/users/ManageIQ/orgs","repos_url":"https://api.github.com/users/ManageIQ/repos","events_url":"https://api.github.com/users/ManageIQ/events{/privacy}","received_events_url":"https://api.github.com/users/ManageIQ/received_events","type":"Organization","site_admin":false},"repo":{"id":20789757,"name":"manageiq","full_name":"ManageIQ/manageiq","owner":{"login":"ManageIQ","id":1125575,"avatar_url":"https://avatars.githubusercontent.com/u/1125575?v=3","gravatar_id":"","url":"https://api.github.com/users/ManageIQ","html_url":"https://github.com/ManageIQ","followers_url":"https://api.github.com/users/ManageIQ/followers","following_url":"https://api.github.com/users/ManageIQ/following{/other_user}","gists_url":"https://api.github.com/users/ManageIQ/gists{/gist_id}","starred_url":"https://api.github.com/users/ManageIQ/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ManageIQ/subscriptions","organizations_url":"https://api.github.com/users/ManageIQ/orgs","repos_url":"https://api.github.com/users/ManageIQ/repos","events_url":"https://api.github.com/users/ManageIQ/events{/privacy}","received_events_url":"https://api.github.com/users/ManageIQ/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/ManageIQ/manageiq","description":"ManageIQ Cloud Management Platform","fork":false,"url":"https://api.github.com/repos/ManageIQ/manageiq","forks_url":"https://api.github.com/repos/ManageIQ/manageiq/forks","keys_url":"https://api.github.com/repos/ManageIQ/manageiq/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ManageIQ/manageiq/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ManageIQ/manageiq/teams","hooks_url":"https://api.github.com/repos/ManageIQ/manageiq/hooks","issue_events_url":"https://api.github.com/repos/ManageIQ/manageiq/issues/events{/number}","events_url":"https://api.github.com/repos/ManageIQ/manageiq/events","assignees_url":"https://api.github.com/repos/ManageIQ/manageiq/assignees{/user}","branches_url":"https://api.github.com/repos/ManageIQ/manageiq/branches{/branch}","tags_url":"https://api.github.com/repos/ManageIQ/manageiq/tags","blobs_url":"https://api.github.com/repos/ManageIQ/manageiq/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ManageIQ/manageiq/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ManageIQ/manageiq/git/refs{/sha}","trees_url":"https://api.github.com/repos/ManageIQ/manageiq/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ManageIQ/manageiq/statuses/{sha}","languages_url":"https://api.github.com/repos/ManageIQ/manageiq/languages","stargazers_url":"https://api.github.com/repos/ManageIQ/manageiq/stargazers","contributors_url":"https://api.github.com/repos/ManageIQ/manageiq/contributors","subscribers_url":"https://api.github.com/repos/ManageIQ/manageiq/subscribers","subscription_url":"https://api.github.com/repos/ManageIQ/manageiq/subscription","commits_url":"https://api.github.com/repos/ManageIQ/manageiq/commits{/sha}","git_commits_url":"https://api.github.com/repos/ManageIQ/manageiq/git/commits{/sha}","comments_url":"https://api.github.com/repos/ManageIQ/manageiq/comments{/number}","issue_comment_url":"https://api.github.com/repos/ManageIQ/manageiq/issues/comments/{number}","contents_url":"https://api.github.com/repos/ManageIQ/manageiq/contents/{+path}","compare_url":"https://api.github.com/repos/ManageIQ/manageiq/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ManageIQ/manageiq/merges","archive_url":"https://api.github.com/repos/ManageIQ/manageiq/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ManageIQ/manageiq/downloads","issues_url":"https://api.github.com/repos/ManageIQ/manageiq/issues{/number}","pulls_url":"https://api.github.com/repos/ManageIQ/manageiq/pulls{/number}","milestones_url":"https://api.github.com/repos/ManageIQ/manageiq/milestones{/number}","notifications_url":"https://api.github.com/repos/ManageIQ/manageiq/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ManageIQ/manageiq/labels{/name}","releases_url":"https://api.github.com/repos/ManageIQ/manageiq/releases{/id}","created_at":"2014-06-13T02:31:26Z","updated_at":"2014-11-10T09:06:58Z","pushed_at":"2014-11-11T11:56:38Z","git_url":"git://github.com/ManageIQ/manageiq.git","ssh_url":"git@github.com:ManageIQ/manageiq.git","clone_url":"https://github.com/ManageIQ/manageiq.git","svn_url":"https://github.com/ManageIQ/manageiq","homepage":"manageiq.org","size":88413,"stargazers_count":65,"watchers_count":65,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":68,"mirror_url":null,"open_issues_count":166,"forks":68,"open_issues":166,"watchers":65,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ManageIQ/manageiq/pulls/1037"},"html":{"href":"https://github.com/ManageIQ/manageiq/pull/1037"},"issue":{"href":"https://api.github.com/repos/ManageIQ/manageiq/issues/1037"},"comments":{"href":"https://api.github.com/repos/ManageIQ/manageiq/issues/1037/comments"},"review_comments":{"href":"https://api.github.com/repos/ManageIQ/manageiq/pulls/1037/comments"},"review_comment":{"href":"https://api.github.com/repos/ManageIQ/manageiq/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/ManageIQ/manageiq/pulls/1037/commits"},"statuses":{"href":"https://api.github.com/repos/ManageIQ/manageiq/statuses/edc3f7e06057ac626de2eabb62809f4894a8e9ff"}}}}