{"action":"created","comment":{"url":"https://api.github.com/repos/ansible/ansible/pulls/comments/239128919","pull_request_review_id":181836286,"id":239128919,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIzOTEyODkxOQ==","diff_hunk":"@@ -73,12 +73,12 @@\n \n import os\n \n+from itertools import product\n+\n from ansible import constants as C\n from ansible.errors import AnsibleParserError\n-from ansible.plugins.cache import FactCache\n from ansible.plugins.inventory import BaseInventoryPlugin\n-\n-from itertools import product\n+from ansible.vars.fact_cache import FactCache","path":"lib/ansible/plugins/inventory/generator.py","position":12,"original_position":12,"commit_id":"0d2b89b04b913dc8f60e7ef7d866007c61b778cb","original_commit_id":"0d2b89b04b913dc8f60e7ef7d866007c61b778cb","user":{"login":"bcoca","id":836171,"node_id":"MDQ6VXNlcjgzNjE3MQ==","avatar_url":"https://avatars1.githubusercontent.com/u/836171?v=4","gravatar_id":"","url":"https://api.github.com/users/bcoca","html_url":"https://github.com/bcoca","followers_url":"https://api.github.com/users/bcoca/followers","following_url":"https://api.github.com/users/bcoca/following{/other_user}","gists_url":"https://api.github.com/users/bcoca/gists{/gist_id}","starred_url":"https://api.github.com/users/bcoca/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bcoca/subscriptions","organizations_url":"https://api.github.com/users/bcoca/orgs","repos_url":"https://api.github.com/users/bcoca/repos","events_url":"https://api.github.com/users/bcoca/events{/privacy}","received_events_url":"https://api.github.com/users/bcoca/received_events","type":"User","site_admin":false},"body":"we can just remove it, it is not really in use","created_at":"2018-12-05T16:05:10Z","updated_at":"2018-12-05T16:17:51Z","html_url":"https://github.com/ansible/ansible/pull/49516#discussion_r239128919","pull_request_url":"https://api.github.com/repos/ansible/ansible/pulls/49516","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/ansible/ansible/pulls/comments/239128919"},"html":{"href":"https://github.com/ansible/ansible/pull/49516#discussion_r239128919"},"pull_request":{"href":"https://api.github.com/repos/ansible/ansible/pulls/49516"}}},"pull_request":{"url":"https://api.github.com/repos/ansible/ansible/pulls/49516","id":236003603,"node_id":"MDExOlB1bGxSZXF1ZXN0MjM2MDAzNjAz","html_url":"https://github.com/ansible/ansible/pull/49516","diff_url":"https://github.com/ansible/ansible/pull/49516.diff","patch_url":"https://github.com/ansible/ansible/pull/49516.patch","issue_url":"https://api.github.com/repos/ansible/ansible/issues/49516","number":49516,"state":"open","locked":false,"title":"Fix fact cache","user":{"login":"abadger","id":209242,"node_id":"MDQ6VXNlcjIwOTI0Mg==","avatar_url":"https://avatars2.githubusercontent.com/u/209242?v=4","gravatar_id":"","url":"https://api.github.com/users/abadger","html_url":"https://github.com/abadger","followers_url":"https://api.github.com/users/abadger/followers","following_url":"https://api.github.com/users/abadger/following{/other_user}","gists_url":"https://api.github.com/users/abadger/gists{/gist_id}","starred_url":"https://api.github.com/users/abadger/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abadger/subscriptions","organizations_url":"https://api.github.com/users/abadger/orgs","repos_url":"https://api.github.com/users/abadger/repos","events_url":"https://api.github.com/users/abadger/events{/privacy}","received_events_url":"https://api.github.com/users/abadger/received_events","type":"User","site_admin":false},"body":"##### SUMMARY\r\n    Fix FactCache.update() to conform to the dict API\r\n    \r\n    * Move ansible.plugins.cache.FactCache to\r\n      ansible.vars.fact_cache.FactCache because this isn't part of the cache\r\n      plugin API.\r\n    * Add backwards compatibility when calling update on the new FactCache\r\n    * Remove code for calling old FactCache. There's no way to call the old\r\n      FactCache so there's no need for backwards compatible code for calling\r\n      code.  Backwards compatibility is handling things which are calling\r\n      the new FactCache.\r\n\r\n##### ISSUE TYPE\r\n<!--- Pick one below and delete the rest -->\r\n- Bugfix Pull Request\r\n\r\n##### COMPONENT NAME\r\nlib/ansible/plugins/cache/__init__.py\r\n\r\n##### ADDITIONAL INFORMATION\r\nThis is a rework of the fact cache as discussed with @nitzmahone @mattclay @sivel and @bcoca.  It addresses most of the problems raised with the previous PR except that it does not handle any potential performance regression (@nitzmahone's critique of how the new update() code interacts with the code in ```VariableManager.set_nonpersistent_facts()``` to call .dict.update() many more times than before.)\r\n\r\n@nitzmahone's critique needs to be resolved before this can be considered ready to merge.\r\n\r\n@sivel pointed out that the original bug report (that we end up calling dict.update(key, value) instead of FactCache.update(key, value) in some cases can be solved in a different way as well: \r\n``` python\r\n# in VariableManager.__init__():\r\n        try:\r\n            self._fact_cache = FactCache()\r\n        except AnsibleError as e:\r\n            display.warning(to_native(e))\r\n            # fallback to memory cache plugin\r\n            C.CACHE_PLUGIN = 'memory'\r\n            self._fact_cache = FactCache()\r\n```\r\n\r\nIf we want to backport a fix for the issue, I think sivel;s method is how we should backport it as it avoids introducing any deprecations into the older releases.  However, sivel's method does not correct the problem of the FactCache advertising that it follows the dict API (by subclassing MutableMapping) while implementing an incompatible update() method.  So we probably should do something more like this in devel.","created_at":"2018-12-05T00:10:46Z","updated_at":"2018-12-05T16:17:51Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a5ef73133f9b1a411bb9375c2322622070a3ee49","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":1043397454,"node_id":"MDU6TGFiZWwxMDQzMzk3NDU0","url":"https://api.github.com/repos/ansible/ansible/labels/affects_2.8","name":"affects_2.8","color":"ededed","default":false},{"id":785271699,"node_id":"MDU6TGFiZWw3ODUyNzE2OTk=","url":"https://api.github.com/repos/ansible/ansible/labels/bug","name":"bug","color":"fbca04","default":true},{"id":849212023,"node_id":"MDU6TGFiZWw4NDkyMTIwMjM=","url":"https://api.github.com/repos/ansible/ansible/labels/docs","name":"docs","color":"0052cc","default":false},{"id":1114914768,"node_id":"MDU6TGFiZWwxMTE0OTE0NzY4","url":"https://api.github.com/repos/ansible/ansible/labels/docsite","name":"docsite","color":"bfd4f2","default":false},{"id":61436281,"node_id":"MDU6TGFiZWw2MTQzNjI4MQ==","url":"https://api.github.com/repos/ansible/ansible/labels/needs_revision","name":"needs_revision","color":"eb6450","default":false},{"id":503572087,"node_id":"MDU6TGFiZWw1MDM1NzIwODc=","url":"https://api.github.com/repos/ansible/ansible/labels/needs_triage","name":"needs_triage","color":"0e8a16","default":false},{"id":420038402,"node_id":"MDU6TGFiZWw0MjAwMzg0MDI=","url":"https://api.github.com/repos/ansible/ansible/labels/performance","name":"performance","color":"555555","default":false},{"id":636327968,"node_id":"MDU6TGFiZWw2MzYzMjc5Njg=","url":"https://api.github.com/repos/ansible/ansible/labels/support:community","name":"support:community","color":"ededed","default":false},{"id":636352796,"node_id":"MDU6TGFiZWw2MzYzNTI3OTY=","url":"https://api.github.com/repos/ansible/ansible/labels/support:core","name":"support:core","color":"ededed","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/ansible/ansible/pulls/49516/commits","review_comments_url":"https://api.github.com/repos/ansible/ansible/pulls/49516/comments","review_comment_url":"https://api.github.com/repos/ansible/ansible/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ansible/ansible/issues/49516/comments","statuses_url":"https://api.github.com/repos/ansible/ansible/statuses/0d2b89b04b913dc8f60e7ef7d866007c61b778cb","head":{"label":"abadger:fix-fact-cache","ref":"fix-fact-cache","sha":"0d2b89b04b913dc8f60e7ef7d866007c61b778cb","user":{"login":"abadger","id":209242,"node_id":"MDQ6VXNlcjIwOTI0Mg==","avatar_url":"https://avatars2.githubusercontent.com/u/209242?v=4","gravatar_id":"","url":"https://api.github.com/users/abadger","html_url":"https://github.com/abadger","followers_url":"https://api.github.com/users/abadger/followers","following_url":"https://api.github.com/users/abadger/following{/other_user}","gists_url":"https://api.github.com/users/abadger/gists{/gist_id}","starred_url":"https://api.github.com/users/abadger/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abadger/subscriptions","organizations_url":"https://api.github.com/users/abadger/orgs","repos_url":"https://api.github.com/users/abadger/repos","events_url":"https://api.github.com/users/abadger/events{/privacy}","received_events_url":"https://api.github.com/users/abadger/received_events","type":"User","site_admin":false},"repo":{"id":17755774,"node_id":"MDEwOlJlcG9zaXRvcnkxNzc1NTc3NA==","name":"ansible","full_name":"abadger/ansible","private":false,"owner":{"login":"abadger","id":209242,"node_id":"MDQ6VXNlcjIwOTI0Mg==","avatar_url":"https://avatars2.githubusercontent.com/u/209242?v=4","gravatar_id":"","url":"https://api.github.com/users/abadger","html_url":"https://github.com/abadger","followers_url":"https://api.github.com/users/abadger/followers","following_url":"https://api.github.com/users/abadger/following{/other_user}","gists_url":"https://api.github.com/users/abadger/gists{/gist_id}","starred_url":"https://api.github.com/users/abadger/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abadger/subscriptions","organizations_url":"https://api.github.com/users/abadger/orgs","repos_url":"https://api.github.com/users/abadger/repos","events_url":"https://api.github.com/users/abadger/events{/privacy}","received_events_url":"https://api.github.com/users/abadger/received_events","type":"User","site_admin":false},"html_url":"https://github.com/abadger/ansible","description":"Ansible is a radically simple IT automation platform that makes your applications and systems easier to deploy. Avoid writing scripts or custom code to deploy and update your applications— automate in a language that approaches plain English, using SSH, with no agents to install on remote systems.","fork":true,"url":"https://api.github.com/repos/abadger/ansible","forks_url":"https://api.github.com/repos/abadger/ansible/forks","keys_url":"https://api.github.com/repos/abadger/ansible/keys{/key_id}","collaborators_url":"https://api.github.com/repos/abadger/ansible/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/abadger/ansible/teams","hooks_url":"https://api.github.com/repos/abadger/ansible/hooks","issue_events_url":"https://api.github.com/repos/abadger/ansible/issues/events{/number}","events_url":"https://api.github.com/repos/abadger/ansible/events","assignees_url":"https://api.github.com/repos/abadger/ansible/assignees{/user}","branches_url":"https://api.github.com/repos/abadger/ansible/branches{/branch}","tags_url":"https://api.github.com/repos/abadger/ansible/tags","blobs_url":"https://api.github.com/repos/abadger/ansible/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/abadger/ansible/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/abadger/ansible/git/refs{/sha}","trees_url":"https://api.github.com/repos/abadger/ansible/git/trees{/sha}","statuses_url":"https://api.github.com/repos/abadger/ansible/statuses/{sha}","languages_url":"https://api.github.com/repos/abadger/ansible/languages","stargazers_url":"https://api.github.com/repos/abadger/ansible/stargazers","contributors_url":"https://api.github.com/repos/abadger/ansible/contributors","subscribers_url":"https://api.github.com/repos/abadger/ansible/subscribers","subscription_url":"https://api.github.com/repos/abadger/ansible/subscription","commits_url":"https://api.github.com/repos/abadger/ansible/commits{/sha}","git_commits_url":"https://api.github.com/repos/abadger/ansible/git/commits{/sha}","comments_url":"https://api.github.com/repos/abadger/ansible/comments{/number}","issue_comment_url":"https://api.github.com/repos/abadger/ansible/issues/comments{/number}","contents_url":"https://api.github.com/repos/abadger/ansible/contents/{+path}","compare_url":"https://api.github.com/repos/abadger/ansible/compare/{base}...{head}","merges_url":"https://api.github.com/repos/abadger/ansible/merges","archive_url":"https://api.github.com/repos/abadger/ansible/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/abadger/ansible/downloads","issues_url":"https://api.github.com/repos/abadger/ansible/issues{/number}","pulls_url":"https://api.github.com/repos/abadger/ansible/pulls{/number}","milestones_url":"https://api.github.com/repos/abadger/ansible/milestones{/number}","notifications_url":"https://api.github.com/repos/abadger/ansible/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/abadger/ansible/labels{/name}","releases_url":"https://api.github.com/repos/abadger/ansible/releases{/id}","deployments_url":"https://api.github.com/repos/abadger/ansible/deployments","created_at":"2014-03-14T18:20:10Z","updated_at":"2018-12-04T22:04:56Z","pushed_at":"2018-12-05T16:03:11Z","git_url":"git://github.com/abadger/ansible.git","ssh_url":"git@github.com:abadger/ansible.git","clone_url":"https://github.com/abadger/ansible.git","svn_url":"https://github.com/abadger/ansible","homepage":"http://ansible.com/","size":144854,"stargazers_count":1,"watchers_count":1,"language":"Python","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"gpl-3.0","name":"GNU General Public License v3.0","spdx_id":"GPL-3.0","url":"https://api.github.com/licenses/gpl-3.0","node_id":"MDc6TGljZW5zZTk="},"forks":1,"open_issues":0,"watchers":1,"default_branch":"devel"}},"base":{"label":"ansible:devel","ref":"devel","sha":"287a42eba0d23b6787796f931d167ca262f2bc96","user":{"login":"ansible","id":1507452,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE1MDc0NTI=","avatar_url":"https://avatars2.githubusercontent.com/u/1507452?v=4","gravatar_id":"","url":"https://api.github.com/users/ansible","html_url":"https://github.com/ansible","followers_url":"https://api.github.com/users/ansible/followers","following_url":"https://api.github.com/users/ansible/following{/other_user}","gists_url":"https://api.github.com/users/ansible/gists{/gist_id}","starred_url":"https://api.github.com/users/ansible/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ansible/subscriptions","organizations_url":"https://api.github.com/users/ansible/orgs","repos_url":"https://api.github.com/users/ansible/repos","events_url":"https://api.github.com/users/ansible/events{/privacy}","received_events_url":"https://api.github.com/users/ansible/received_events","type":"Organization","site_admin":false},"repo":{"id":3638964,"node_id":"MDEwOlJlcG9zaXRvcnkzNjM4OTY0","name":"ansible","full_name":"ansible/ansible","private":false,"owner":{"login":"ansible","id":1507452,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE1MDc0NTI=","avatar_url":"https://avatars2.githubusercontent.com/u/1507452?v=4","gravatar_id":"","url":"https://api.github.com/users/ansible","html_url":"https://github.com/ansible","followers_url":"https://api.github.com/users/ansible/followers","following_url":"https://api.github.com/users/ansible/following{/other_user}","gists_url":"https://api.github.com/users/ansible/gists{/gist_id}","starred_url":"https://api.github.com/users/ansible/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ansible/subscriptions","organizations_url":"https://api.github.com/users/ansible/orgs","repos_url":"https://api.github.com/users/ansible/repos","events_url":"https://api.github.com/users/ansible/events{/privacy}","received_events_url":"https://api.github.com/users/ansible/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/ansible/ansible","description":"Ansible is a radically simple IT automation platform that makes your applications and systems easier to deploy. Avoid writing scripts or custom code to deploy and update your applications — automate in a language that approaches plain English, using SSH, with no agents to install on remote systems. https://docs.ansible.com/ansible/","fork":false,"url":"https://api.github.com/repos/ansible/ansible","forks_url":"https://api.github.com/repos/ansible/ansible/forks","keys_url":"https://api.github.com/repos/ansible/ansible/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ansible/ansible/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ansible/ansible/teams","hooks_url":"https://api.github.com/repos/ansible/ansible/hooks","issue_events_url":"https://api.github.com/repos/ansible/ansible/issues/events{/number}","events_url":"https://api.github.com/repos/ansible/ansible/events","assignees_url":"https://api.github.com/repos/ansible/ansible/assignees{/user}","branches_url":"https://api.github.com/repos/ansible/ansible/branches{/branch}","tags_url":"https://api.github.com/repos/ansible/ansible/tags","blobs_url":"https://api.github.com/repos/ansible/ansible/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ansible/ansible/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ansible/ansible/git/refs{/sha}","trees_url":"https://api.github.com/repos/ansible/ansible/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ansible/ansible/statuses/{sha}","languages_url":"https://api.github.com/repos/ansible/ansible/languages","stargazers_url":"https://api.github.com/repos/ansible/ansible/stargazers","contributors_url":"https://api.github.com/repos/ansible/ansible/contributors","subscribers_url":"https://api.github.com/repos/ansible/ansible/subscribers","subscription_url":"https://api.github.com/repos/ansible/ansible/subscription","commits_url":"https://api.github.com/repos/ansible/ansible/commits{/sha}","git_commits_url":"https://api.github.com/repos/ansible/ansible/git/commits{/sha}","comments_url":"https://api.github.com/repos/ansible/ansible/comments{/number}","issue_comment_url":"https://api.github.com/repos/ansible/ansible/issues/comments{/number}","contents_url":"https://api.github.com/repos/ansible/ansible/contents/{+path}","compare_url":"https://api.github.com/repos/ansible/ansible/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ansible/ansible/merges","archive_url":"https://api.github.com/repos/ansible/ansible/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ansible/ansible/downloads","issues_url":"https://api.github.com/repos/ansible/ansible/issues{/number}","pulls_url":"https://api.github.com/repos/ansible/ansible/pulls{/number}","milestones_url":"https://api.github.com/repos/ansible/ansible/milestones{/number}","notifications_url":"https://api.github.com/repos/ansible/ansible/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ansible/ansible/labels{/name}","releases_url":"https://api.github.com/repos/ansible/ansible/releases{/id}","deployments_url":"https://api.github.com/repos/ansible/ansible/deployments","created_at":"2012-03-06T14:58:02Z","updated_at":"2018-12-05T16:15:07Z","pushed_at":"2018-12-05T16:03:44Z","git_url":"git://github.com/ansible/ansible.git","ssh_url":"git@github.com:ansible/ansible.git","clone_url":"https://github.com/ansible/ansible.git","svn_url":"https://github.com/ansible/ansible","homepage":"https://www.ansible.com/","size":147116,"stargazers_count":34115,"watchers_count":34115,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":13677,"mirror_url":null,"archived":false,"open_issues_count":5539,"license":{"key":"gpl-3.0","name":"GNU General Public License v3.0","spdx_id":"GPL-3.0","url":"https://api.github.com/licenses/gpl-3.0","node_id":"MDc6TGljZW5zZTk="},"forks":13677,"open_issues":5539,"watchers":34115,"default_branch":"devel"}},"_links":{"self":{"href":"https://api.github.com/repos/ansible/ansible/pulls/49516"},"html":{"href":"https://github.com/ansible/ansible/pull/49516"},"issue":{"href":"https://api.github.com/repos/ansible/ansible/issues/49516"},"comments":{"href":"https://api.github.com/repos/ansible/ansible/issues/49516/comments"},"review_comments":{"href":"https://api.github.com/repos/ansible/ansible/pulls/49516/comments"},"review_comment":{"href":"https://api.github.com/repos/ansible/ansible/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ansible/ansible/pulls/49516/commits"},"statuses":{"href":"https://api.github.com/repos/ansible/ansible/statuses/0d2b89b04b913dc8f60e7ef7d866007c61b778cb"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"bcoca"}}