{"action":"created","comment":{"url":"https://api.github.com/repos/elastic/elasticsearch/pulls/comments/36607311","id":36607311,"diff_hunk":"@@ -20,9 +20,19 @@\n package org.elasticsearch.common.inject;\n \n /**\n+ * This interface can be added to a Module to spawn sub modules. DO NOT USE.\n  *\n+ * This is fundamentally broken.\n+ * <ul>\n+ * <li>If you have a plugin with multiple modules, return all the modules at once.</li>\n+ * <li>If you are trying to make the implementation of a module \"pluggable\", don't do it.\n+ * This is not extendable because custom implementations (using onModule) cannot be\n+ * registered before spawnModules() is called.</li>\n+ * </ul>\n+ * @deprecated Create modules directly\n  */\n-public interface SpawnModules {\n+@Deprecated","path":"core/src/main/java/org/elasticsearch/common/inject/SpawnShittyModules.java","position":16,"original_position":16,"commit_id":"3cfe06bab1a0b71edc043ef2c0fcb452a6b84622","original_commit_id":"3cfe06bab1a0b71edc043ef2c0fcb452a6b84622","user":{"login":"rjernst","id":289412,"avatar_url":"https://avatars.githubusercontent.com/u/289412?v=3","gravatar_id":"","url":"https://api.github.com/users/rjernst","html_url":"https://github.com/rjernst","followers_url":"https://api.github.com/users/rjernst/followers","following_url":"https://api.github.com/users/rjernst/following{/other_user}","gists_url":"https://api.github.com/users/rjernst/gists{/gist_id}","starred_url":"https://api.github.com/users/rjernst/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rjernst/subscriptions","organizations_url":"https://api.github.com/users/rjernst/orgs","repos_url":"https://api.github.com/users/rjernst/repos","events_url":"https://api.github.com/users/rjernst/events{/privacy}","received_events_url":"https://api.github.com/users/rjernst/received_events","type":"User","site_admin":false},"body":"> if removing the crappy api introduces another crappy thing (api/design/etc..)\r\n\r\nBut that's not the case here. Flattening modules is a good thing. It will make dependencies of services/components much less \"spaghetti\". Obviously I can't do flattening/cleanup in this PR. But the deprecation is here to warn against adding more of this craziness.\r\n\r\n> The problem here is that there needs to be a well defined function for replacing modules, on the container level..\r\n\r\nI disagree. There should be no need to replace a module, there is no need to make every part of Elasticsearch pluggable. We should have very specific pieces that are overrideable. Using the DiscoveryModule as an example, we have \"local\" and \"zen\" discovery types that are \"builtin\" to ES. These are always available, and zen has a number of associated classes that are bound in discovery module. However, if a plugin wants to add a new discovery type, the Discovery class is the only thing they are plugging in, and that is registerable with the DiscoveryModule. If they require extra bindings, they can do so in their own module, which is added as part of their plugin. All of our extension points should work this way. ","created_at":"2015-08-10T07:32:37Z","updated_at":"2015-08-10T07:32:37Z","html_url":"https://github.com/elastic/elasticsearch/pull/12744#discussion_r36607311","pull_request_url":"https://api.github.com/repos/elastic/elasticsearch/pulls/12744","_links":{"self":{"href":"https://api.github.com/repos/elastic/elasticsearch/pulls/comments/36607311"},"html":{"href":"https://github.com/elastic/elasticsearch/pull/12744#discussion_r36607311"},"pull_request":{"href":"https://api.github.com/repos/elastic/elasticsearch/pulls/12744"}}},"pull_request":{"url":"https://api.github.com/repos/elastic/elasticsearch/pulls/12744","id":41962144,"html_url":"https://github.com/elastic/elasticsearch/pull/12744","diff_url":"https://github.com/elastic/elasticsearch/pull/12744.diff","patch_url":"https://github.com/elastic/elasticsearch/pull/12744.patch","issue_url":"https://api.github.com/repos/elastic/elasticsearch/issues/12744","number":12744,"state":"open","locked":false,"title":"Remove Settings.getAsClass","user":{"login":"rjernst","id":289412,"avatar_url":"https://avatars.githubusercontent.com/u/289412?v=3","gravatar_id":"","url":"https://api.github.com/users/rjernst","html_url":"https://github.com/rjernst","followers_url":"https://api.github.com/users/rjernst/followers","following_url":"https://api.github.com/users/rjernst/following{/other_user}","gists_url":"https://api.github.com/users/rjernst/gists{/gist_id}","starred_url":"https://api.github.com/users/rjernst/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rjernst/subscriptions","organizations_url":"https://api.github.com/users/rjernst/orgs","repos_url":"https://api.github.com/users/rjernst/repos","events_url":"https://api.github.com/users/rjernst/events{/privacy}","received_events_url":"https://api.github.com/users/rjernst/received_events","type":"User","site_admin":false},"body":"This method on settings loaded a class, based on a setting value, using the default classloader. It had all kinds of leniency in how the classname was found, and simply cannot work with plugins having isolated classloaders.\r\n\r\nThis change removes that method. Some of the uses of it were for custom extension points, like custom repository or discovery types. A lot were just there to plugin mock implementations for tests. For the settings that were legitimate, all now support plugins adding the given setting via onModule. For those that were specific to tests for mocks, they now use Classes.loadClass (a helper around Class.forName). This is a temporary measure until (in a future PR) tests can change the implementation via package private statics.\r\n\r\nI also removed a number of unnecessary intermediate modules, added a \"jvm-example\" plugin that can be filled in in the future as a smoke test for breaking plugins, and gave some documentation to \"spawn\" modules interface.\r\n\r\ncloses #12643\r\ncloses #12656","created_at":"2015-08-08T23:00:48Z","updated_at":"2015-08-10T07:32:37Z","closed_at":null,"merged_at":null,"merge_commit_sha":"47435ddb1b4db51ec57a3fc32b6bf4ee55cecab6","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/elastic/elasticsearch/pulls/12744/commits","review_comments_url":"https://api.github.com/repos/elastic/elasticsearch/pulls/12744/comments","review_comment_url":"https://api.github.com/repos/elastic/elasticsearch/pulls/comments{/number}","comments_url":"https://api.github.com/repos/elastic/elasticsearch/issues/12744/comments","statuses_url":"https://api.github.com/repos/elastic/elasticsearch/statuses/3cfe06bab1a0b71edc043ef2c0fcb452a6b84622","head":{"label":"rjernst:hellspawn","ref":"hellspawn","sha":"3cfe06bab1a0b71edc043ef2c0fcb452a6b84622","user":{"login":"rjernst","id":289412,"avatar_url":"https://avatars.githubusercontent.com/u/289412?v=3","gravatar_id":"","url":"https://api.github.com/users/rjernst","html_url":"https://github.com/rjernst","followers_url":"https://api.github.com/users/rjernst/followers","following_url":"https://api.github.com/users/rjernst/following{/other_user}","gists_url":"https://api.github.com/users/rjernst/gists{/gist_id}","starred_url":"https://api.github.com/users/rjernst/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rjernst/subscriptions","organizations_url":"https://api.github.com/users/rjernst/orgs","repos_url":"https://api.github.com/users/rjernst/repos","events_url":"https://api.github.com/users/rjernst/events{/privacy}","received_events_url":"https://api.github.com/users/rjernst/received_events","type":"User","site_admin":false},"repo":{"id":21088403,"name":"elasticsearch","full_name":"rjernst/elasticsearch","owner":{"login":"rjernst","id":289412,"avatar_url":"https://avatars.githubusercontent.com/u/289412?v=3","gravatar_id":"","url":"https://api.github.com/users/rjernst","html_url":"https://github.com/rjernst","followers_url":"https://api.github.com/users/rjernst/followers","following_url":"https://api.github.com/users/rjernst/following{/other_user}","gists_url":"https://api.github.com/users/rjernst/gists{/gist_id}","starred_url":"https://api.github.com/users/rjernst/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rjernst/subscriptions","organizations_url":"https://api.github.com/users/rjernst/orgs","repos_url":"https://api.github.com/users/rjernst/repos","events_url":"https://api.github.com/users/rjernst/events{/privacy}","received_events_url":"https://api.github.com/users/rjernst/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/rjernst/elasticsearch","description":"Open Source, Distributed, RESTful Search Engine","fork":true,"url":"https://api.github.com/repos/rjernst/elasticsearch","forks_url":"https://api.github.com/repos/rjernst/elasticsearch/forks","keys_url":"https://api.github.com/repos/rjernst/elasticsearch/keys{/key_id}","collaborators_url":"https://api.github.com/repos/rjernst/elasticsearch/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/rjernst/elasticsearch/teams","hooks_url":"https://api.github.com/repos/rjernst/elasticsearch/hooks","issue_events_url":"https://api.github.com/repos/rjernst/elasticsearch/issues/events{/number}","events_url":"https://api.github.com/repos/rjernst/elasticsearch/events","assignees_url":"https://api.github.com/repos/rjernst/elasticsearch/assignees{/user}","branches_url":"https://api.github.com/repos/rjernst/elasticsearch/branches{/branch}","tags_url":"https://api.github.com/repos/rjernst/elasticsearch/tags","blobs_url":"https://api.github.com/repos/rjernst/elasticsearch/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/rjernst/elasticsearch/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/rjernst/elasticsearch/git/refs{/sha}","trees_url":"https://api.github.com/repos/rjernst/elasticsearch/git/trees{/sha}","statuses_url":"https://api.github.com/repos/rjernst/elasticsearch/statuses/{sha}","languages_url":"https://api.github.com/repos/rjernst/elasticsearch/languages","stargazers_url":"https://api.github.com/repos/rjernst/elasticsearch/stargazers","contributors_url":"https://api.github.com/repos/rjernst/elasticsearch/contributors","subscribers_url":"https://api.github.com/repos/rjernst/elasticsearch/subscribers","subscription_url":"https://api.github.com/repos/rjernst/elasticsearch/subscription","commits_url":"https://api.github.com/repos/rjernst/elasticsearch/commits{/sha}","git_commits_url":"https://api.github.com/repos/rjernst/elasticsearch/git/commits{/sha}","comments_url":"https://api.github.com/repos/rjernst/elasticsearch/comments{/number}","issue_comment_url":"https://api.github.com/repos/rjernst/elasticsearch/issues/comments{/number}","contents_url":"https://api.github.com/repos/rjernst/elasticsearch/contents/{+path}","compare_url":"https://api.github.com/repos/rjernst/elasticsearch/compare/{base}...{head}","merges_url":"https://api.github.com/repos/rjernst/elasticsearch/merges","archive_url":"https://api.github.com/repos/rjernst/elasticsearch/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/rjernst/elasticsearch/downloads","issues_url":"https://api.github.com/repos/rjernst/elasticsearch/issues{/number}","pulls_url":"https://api.github.com/repos/rjernst/elasticsearch/pulls{/number}","milestones_url":"https://api.github.com/repos/rjernst/elasticsearch/milestones{/number}","notifications_url":"https://api.github.com/repos/rjernst/elasticsearch/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/rjernst/elasticsearch/labels{/name}","releases_url":"https://api.github.com/repos/rjernst/elasticsearch/releases{/id}","created_at":"2014-06-22T07:43:32Z","updated_at":"2014-10-12T08:10:41Z","pushed_at":"2015-08-08T22:51:35Z","git_url":"git://github.com/rjernst/elasticsearch.git","ssh_url":"git@github.com:rjernst/elasticsearch.git","clone_url":"https://github.com/rjernst/elasticsearch.git","svn_url":"https://github.com/rjernst/elasticsearch","homepage":"http://elasticsearch.org","size":147594,"stargazers_count":0,"watchers_count":0,"language":"Java","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":"elastic:master","ref":"master","sha":"660e376be17dc113c7a298d6eeb6df99f6d21215","user":{"login":"elastic","id":6764390,"avatar_url":"https://avatars.githubusercontent.com/u/6764390?v=3","gravatar_id":"","url":"https://api.github.com/users/elastic","html_url":"https://github.com/elastic","followers_url":"https://api.github.com/users/elastic/followers","following_url":"https://api.github.com/users/elastic/following{/other_user}","gists_url":"https://api.github.com/users/elastic/gists{/gist_id}","starred_url":"https://api.github.com/users/elastic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/elastic/subscriptions","organizations_url":"https://api.github.com/users/elastic/orgs","repos_url":"https://api.github.com/users/elastic/repos","events_url":"https://api.github.com/users/elastic/events{/privacy}","received_events_url":"https://api.github.com/users/elastic/received_events","type":"Organization","site_admin":false},"repo":{"id":507775,"name":"elasticsearch","full_name":"elastic/elasticsearch","owner":{"login":"elastic","id":6764390,"avatar_url":"https://avatars.githubusercontent.com/u/6764390?v=3","gravatar_id":"","url":"https://api.github.com/users/elastic","html_url":"https://github.com/elastic","followers_url":"https://api.github.com/users/elastic/followers","following_url":"https://api.github.com/users/elastic/following{/other_user}","gists_url":"https://api.github.com/users/elastic/gists{/gist_id}","starred_url":"https://api.github.com/users/elastic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/elastic/subscriptions","organizations_url":"https://api.github.com/users/elastic/orgs","repos_url":"https://api.github.com/users/elastic/repos","events_url":"https://api.github.com/users/elastic/events{/privacy}","received_events_url":"https://api.github.com/users/elastic/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/elastic/elasticsearch","description":"Open Source, Distributed, RESTful Search Engine","fork":false,"url":"https://api.github.com/repos/elastic/elasticsearch","forks_url":"https://api.github.com/repos/elastic/elasticsearch/forks","keys_url":"https://api.github.com/repos/elastic/elasticsearch/keys{/key_id}","collaborators_url":"https://api.github.com/repos/elastic/elasticsearch/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/elastic/elasticsearch/teams","hooks_url":"https://api.github.com/repos/elastic/elasticsearch/hooks","issue_events_url":"https://api.github.com/repos/elastic/elasticsearch/issues/events{/number}","events_url":"https://api.github.com/repos/elastic/elasticsearch/events","assignees_url":"https://api.github.com/repos/elastic/elasticsearch/assignees{/user}","branches_url":"https://api.github.com/repos/elastic/elasticsearch/branches{/branch}","tags_url":"https://api.github.com/repos/elastic/elasticsearch/tags","blobs_url":"https://api.github.com/repos/elastic/elasticsearch/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/elastic/elasticsearch/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/elastic/elasticsearch/git/refs{/sha}","trees_url":"https://api.github.com/repos/elastic/elasticsearch/git/trees{/sha}","statuses_url":"https://api.github.com/repos/elastic/elasticsearch/statuses/{sha}","languages_url":"https://api.github.com/repos/elastic/elasticsearch/languages","stargazers_url":"https://api.github.com/repos/elastic/elasticsearch/stargazers","contributors_url":"https://api.github.com/repos/elastic/elasticsearch/contributors","subscribers_url":"https://api.github.com/repos/elastic/elasticsearch/subscribers","subscription_url":"https://api.github.com/repos/elastic/elasticsearch/subscription","commits_url":"https://api.github.com/repos/elastic/elasticsearch/commits{/sha}","git_commits_url":"https://api.github.com/repos/elastic/elasticsearch/git/commits{/sha}","comments_url":"https://api.github.com/repos/elastic/elasticsearch/comments{/number}","issue_comment_url":"https://api.github.com/repos/elastic/elasticsearch/issues/comments{/number}","contents_url":"https://api.github.com/repos/elastic/elasticsearch/contents/{+path}","compare_url":"https://api.github.com/repos/elastic/elasticsearch/compare/{base}...{head}","merges_url":"https://api.github.com/repos/elastic/elasticsearch/merges","archive_url":"https://api.github.com/repos/elastic/elasticsearch/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/elastic/elasticsearch/downloads","issues_url":"https://api.github.com/repos/elastic/elasticsearch/issues{/number}","pulls_url":"https://api.github.com/repos/elastic/elasticsearch/pulls{/number}","milestones_url":"https://api.github.com/repos/elastic/elasticsearch/milestones{/number}","notifications_url":"https://api.github.com/repos/elastic/elasticsearch/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/elastic/elasticsearch/labels{/name}","releases_url":"https://api.github.com/repos/elastic/elasticsearch/releases{/id}","created_at":"2010-02-08T13:20:56Z","updated_at":"2015-08-10T07:16:20Z","pushed_at":"2015-08-09T23:34:03Z","git_url":"git://github.com/elastic/elasticsearch.git","ssh_url":"git@github.com:elastic/elasticsearch.git","clone_url":"https://github.com/elastic/elasticsearch.git","svn_url":"https://github.com/elastic/elasticsearch","homepage":"https://www.elastic.co/products/elasticsearch","size":393293,"stargazers_count":12280,"watchers_count":12280,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":3943,"mirror_url":null,"open_issues_count":1203,"forks":3943,"open_issues":1203,"watchers":12280,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/elastic/elasticsearch/pulls/12744"},"html":{"href":"https://github.com/elastic/elasticsearch/pull/12744"},"issue":{"href":"https://api.github.com/repos/elastic/elasticsearch/issues/12744"},"comments":{"href":"https://api.github.com/repos/elastic/elasticsearch/issues/12744/comments"},"review_comments":{"href":"https://api.github.com/repos/elastic/elasticsearch/pulls/12744/comments"},"review_comment":{"href":"https://api.github.com/repos/elastic/elasticsearch/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/elastic/elasticsearch/pulls/12744/commits"},"statuses":{"href":"https://api.github.com/repos/elastic/elasticsearch/statuses/3cfe06bab1a0b71edc043ef2c0fcb452a6b84622"}}}}