{"action":"created","issue":{"url":"https://api.github.com/repos/hortonworks/ansible-hortonworks/issues/88","repository_url":"https://api.github.com/repos/hortonworks/ansible-hortonworks","labels_url":"https://api.github.com/repos/hortonworks/ansible-hortonworks/issues/88/labels{/name}","comments_url":"https://api.github.com/repos/hortonworks/ansible-hortonworks/issues/88/comments","events_url":"https://api.github.com/repos/hortonworks/ansible-hortonworks/issues/88/events","html_url":"https://github.com/hortonworks/ansible-hortonworks/pull/88","id":372095449,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI0Mzk0MTQz","number":88,"title":"Redefine how variables are being used for the ability to override ","user":{"login":"zer0glitch","id":4751686,"node_id":"MDQ6VXNlcjQ3NTE2ODY=","avatar_url":"https://avatars2.githubusercontent.com/u/4751686?v=4","gravatar_id":"","url":"https://api.github.com/users/zer0glitch","html_url":"https://github.com/zer0glitch","followers_url":"https://api.github.com/users/zer0glitch/followers","following_url":"https://api.github.com/users/zer0glitch/following{/other_user}","gists_url":"https://api.github.com/users/zer0glitch/gists{/gist_id}","starred_url":"https://api.github.com/users/zer0glitch/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/zer0glitch/subscriptions","organizations_url":"https://api.github.com/users/zer0glitch/orgs","repos_url":"https://api.github.com/users/zer0glitch/repos","events_url":"https://api.github.com/users/zer0glitch/events{/privacy}","received_events_url":"https://api.github.com/users/zer0glitch/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-10-19T19:56:04Z","updated_at":"2018-11-02T16:17:39Z","closed_at":"2018-11-02T16:17:39Z","author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/hortonworks/ansible-hortonworks/pulls/88","html_url":"https://github.com/hortonworks/ansible-hortonworks/pull/88","diff_url":"https://github.com/hortonworks/ansible-hortonworks/pull/88.diff","patch_url":"https://github.com/hortonworks/ansible-hortonworks/pull/88.patch"},"body":"Adding the ability to override variables in the roles.  \r\n\r\n. Code sets the same variables as <variable>_default\r\n. Then inside the task it sets the <variable> to the <variable> if it is defined otherwise <variable>_default that was defined inside the roles vars directory."},"comment":{"url":"https://api.github.com/repos/hortonworks/ansible-hortonworks/issues/comments/435433327","html_url":"https://github.com/hortonworks/ansible-hortonworks/pull/88#issuecomment-435433327","issue_url":"https://api.github.com/repos/hortonworks/ansible-hortonworks/issues/88","id":435433327,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNTQzMzMyNw==","user":{"login":"alexandruanghel","id":5119993,"node_id":"MDQ6VXNlcjUxMTk5OTM=","avatar_url":"https://avatars0.githubusercontent.com/u/5119993?v=4","gravatar_id":"","url":"https://api.github.com/users/alexandruanghel","html_url":"https://github.com/alexandruanghel","followers_url":"https://api.github.com/users/alexandruanghel/followers","following_url":"https://api.github.com/users/alexandruanghel/following{/other_user}","gists_url":"https://api.github.com/users/alexandruanghel/gists{/gist_id}","starred_url":"https://api.github.com/users/alexandruanghel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexandruanghel/subscriptions","organizations_url":"https://api.github.com/users/alexandruanghel/orgs","repos_url":"https://api.github.com/users/alexandruanghel/repos","events_url":"https://api.github.com/users/alexandruanghel/events{/privacy}","received_events_url":"https://api.github.com/users/alexandruanghel/received_events","type":"User","site_admin":false},"created_at":"2018-11-02T16:17:39Z","updated_at":"2018-11-02T16:17:39Z","author_association":"COLLABORATOR","body":"Hi there,\r\n\r\nI'm afraid I cannot accept this PR due to the following reasons:\r\n\r\n`1)` The description of this PR (`Adding the ability to override variables in the roles`) does not correspond with the changes made via the 11 commits you're submitting.\r\n\r\nThere are many other unrelated changes, some of which are even a bit counter-intuitive or I'm not sure what they trying to achieve since there's no description.\r\n\r\nDetails below:\r\n`1.1)` Moving of the \"Fail if a cluster already exists\": https://github.com/hortonworks/ansible-hortonworks/pull/88/files#diff-73862e6fe06f8902b8f5a829c5368a34L48 doesn't make much sense unless \"Check if a cluster already exists\" is also moved.\r\nThe original idea was that if a cluster already exists, there's no point going forward with generating the blueprint and so on, but I can see why you'd want to move the failure (to test variations in the blueprint even if the cluster is already built).\r\n\r\nI would still like to keep as it is as I'm thinking of adding another option to change the `apply_blueprint` behaviour so it skips all the blueprint generation if the cluster exists and still finishes the playbook successfully rather than with a failure.\r\n\r\n\r\n`1.2)` I left the spaces in the body request for very good reasons https://github.com/hortonworks/ansible-hortonworks/pull/88/files#diff-73862e6fe06f8902b8f5a829c5368a34L97 (I'll add some comments as those spaces are indeed they're confusing).\r\nI had to play a lot with strings/jsons/yaml format for both dynamic and static blueprints. Removing the space might work for dynamic blueprints but it will break static blueprint deployments, see #66 \r\n\r\n\r\n`1.3)` The `repository_versions` additions doesn't achieve anything: https://github.com/hortonworks/ansible-hortonworks/pull/88/files#diff-73862e6fe06f8902b8f5a829c5368a34R116\r\nRepositories are handled in the ambari-config role (the ambari-blueprint should only handle 1 thing: generate and apply blueprint). Repositories for both Ambari <2.6 and >=2.6 are being handled in that role: https://github.com/hortonworks/ansible-hortonworks/blob/master/playbooks/roles/ambari-config/tasks/main.yml#L141\r\n\r\nAnd as far as I know that API you're using has been deprecated in Ambari 2.6, being replaced by the VDFs for blueprint deployments: https://docs.hortonworks.com/HDPDocuments/Ambari-2.6.0.0/bk_ambari-release-notes/content/ambari_relnotes-2.6.0.0-behavioral-changes.html\r\nAlso, no need to restart Ambari if a repo information was updated.\r\n\r\nPlease double check the `ambari-repo` role and let me know in a separate issue ticket if that doesn't suffice your use case or you find any issues with it.\r\n\r\n\r\n`1.4)` Not sure what `blueprint_config_options` are for, this is the only place I've seen this variable: https://github.com/hortonworks/ansible-hortonworks/pull/88/files#diff-8f39d280f6c86c8a930d3d1163858c60R429\r\n\r\n\r\n`1.5)` Not sure why you'd want to change the repo id and its uniqueness in the VDF: https://github.com/hortonworks/ansible-hortonworks/pull/88/files#diff-364a6b2dfcf2f9986c63a42fa836dff3R5\r\nI'm using the official VDFs from http://public-repo-1.hortonworks.com/HDP/hdp_urlinfo.json and I'd be reluctant to change anything without a good reason, especially the `<unique>` attribute.\r\n\r\n\r\n`1.6)` Thanks for pointing out the Ambari yum gpg key handling: https://github.com/hortonworks/ansible-hortonworks/pull/88/files#diff-ffee5710d7313c18633140cbc654b5f7R21\r\nInstead of that, I will want to use a gpg key supplied in the playbooks, like it's currently done with Ubuntu: https://github.com/hortonworks/ansible-hortonworks/commit/31e6a07d10db740f10ee5917df27b8515b474510\r\nThis removes the dependency with having a key stored online, plus the gpg should always be used anyway.\r\n\r\n\r\n`1.7)` I see some Accumulo variables but they are not used anywhere else: https://github.com/hortonworks/ansible-hortonworks/pull/88/files#diff-92e5b247d9c21e01ece42267ab5ac4d8R74\r\nAccumulo support was added in https://github.com/hortonworks/ansible-hortonworks/commit/3790deb372d841b28ae8f3657c343e41648de269\r\n\r\n\r\n`2)` I can't see a good reason for adding the `_default` variables and then using the `variable  |  default(variable_default)` construct.\r\nThose variables are themselves the default role variables (even if they're not in the defaults folder due to using `include_vars` for the multiples OS-es).\r\n\r\nAs per Ansible [Variable Precedence](https://docs.ansible.com/ansible/latest/user_guide/playbooks_variables.html#variable-precedence-where-should-i-put-a-variable) those variables can be overridden fairly easy in two ways (which for me they seem sufficient, not sure if you had something else in mind):\r\n- with `extra-vars` command line option\r\n- when the role is applied: https://github.com/hortonworks/ansible-hortonworks/blob/master/playbooks/apply_blueprint.yml#L17\r\nSorry I have no example in these playbooks (sounds like it's a good reason to add one!) but in the [Ansible docs](https://docs.ansible.com/ansible/latest/user_guide/playbooks_reuse_roles.html) you can see some examples:\r\n```\r\n- hosts: webservers\r\n  roles:\r\n    - role: foo_app_instance\r\n      vars:\r\n         dir: '/opt/a'\r\n         app_port: 5000\r\n```\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n"}}
{"actor":{"display_login":"alexandruanghel"}}