)]}'
{".zuul.yaml":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"51c34ae221067e1a0a6431a68a3585e79fd02ea0","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    failure-url: http://zuulv3-dev.openstack.org/logs/{build.uuid}/"},{"line_number":7,"context_line":"    timeout: 1800"},{"line_number":8,"context_line":"    vars:"},{"line_number":9,"context_line":"      zuul_workspace_root: /home/zuul"},{"line_number":10,"context_line":"    nodes:"},{"line_number":11,"context_line":"      - name: ubuntu-xenial"},{"line_number":12,"context_line":"        image: ubuntu-xenial"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_f3254ac2","line":9,"updated":"2017-03-17 22:17:36.000000000","message":"Should this have /workspace on the end?","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":4162,"name":"Paul Belanger","email":"pabelanger@redhat.com","username":"pabelanger"},"change_message_id":"84fe7ed86e99a849d156ba860f6013dbfb792e0b","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    failure-url: http://zuulv3-dev.openstack.org/logs/{build.uuid}/"},{"line_number":7,"context_line":"    timeout: 1800"},{"line_number":8,"context_line":"    vars:"},{"line_number":9,"context_line":"      zuul_workspace_root: /home/zuul"},{"line_number":10,"context_line":"    nodes:"},{"line_number":11,"context_line":"      - name: ubuntu-xenial"},{"line_number":12,"context_line":"        image: ubuntu-xenial"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_27965b93","line":9,"updated":"2017-03-21 16:08:12.000000000","message":"maybe? with zuulv3, do we really need a workspace subdirectory or can we live using /home/zuul? I can add it back, but was trying to save some char length.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":6589,"name":"Jesse Keating","email":"jkeating@j2solutions.net","username":"jesse-keating"},"change_message_id":"58d3b1c6ced0555e790493b958600e7057fd0c4e","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    failure-url: http://zuulv3-dev.openstack.org/logs/{build.uuid}/"},{"line_number":7,"context_line":"    timeout: 1800"},{"line_number":8,"context_line":"    vars:"},{"line_number":9,"context_line":"      zuul_workspace_root: /home/zuul"},{"line_number":10,"context_line":"    nodes:"},{"line_number":11,"context_line":"      - name: ubuntu-xenial"},{"line_number":12,"context_line":"        image: ubuntu-xenial"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_9b714de3","line":9,"in_reply_to":"1a1ced50_f3254ac2","updated":"2017-03-21 00:17:19.000000000","message":"What drives the decision of what path to put in here? Is _this_ the decision point, that doesn\u0027t rely on anything else pre-existing?","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"}],"playbooks/base-pre.yaml":[{"author":{"_account_id":6589,"name":"Jesse Keating","email":"jkeating@j2solutions.net","username":"jesse-keating"},"change_message_id":"58d3b1c6ced0555e790493b958600e7057fd0c4e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"- hosts: all"},{"line_number":2,"context_line":"  vars:"},{"line_number":3,"context_line":"    prepare_workspace_workspace_root: \"{{ zuul_workspace_root }}\""},{"line_number":4,"context_line":"  roles:"},{"line_number":5,"context_line":"    - prepare-workspace"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_9b486d89","line":3,"updated":"2017-03-21 00:17:19.000000000","message":"It would seem more natural to scope this variable down at the role application. Even though this play is a single role, it is still a better practice to scope it down to the role itself rather than the entire play. Since it seems the attempt is to keep the variable only available in the role itself.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"}],"playbooks/roles/prepare-workspace/defaults/main.yaml":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"cd6126fbeed28932a27139e00eb560066f10c708","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prepare_workspace_workspace_root: /home/zuul/workspace"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_ae444974","line":2,"updated":"2017-03-17 22:56:49.000000000","message":"I think that\u0027s reasonable.  The goal is to make things more comprehensible.  Like most strict rules, there may be some situations where it results in the opposite, and I think that\u0027s what we\u0027re seeing here.  \"prepare_workspace_workspace_root\" is very confusing, and in order to make it less confusing, we considered changing the name of the role -- however, that made that *role* more confusing.  That\u0027s why I think this merits re-examining the rule to see if it\u0027s a good fit here.\n\nI think the workspace root is going to be used by *a lot* of roles, and I think passing it around to each of them is going to make for a lot of boilerplate and will turn people off from wanting to write jobs.  That\u0027s why my preference is to just accept that this one should be global.\n\nOne of the things I\u0027m sure we both would like to avoid, however, is something like this leaking into non-zuul-specific playbook content.  For instance, if a devstack deployment script directly referenced zuul_workspace_root, that would be unfortunate, because while it may *want* a working directory, there\u0027s nothing zuul-specific about it, and that\u0027s definitely a case where I agree we should have a \"devstack_workspace_root: {{zuul_workspace_root}}\" variable definition.\n\nI\u0027m very sympathetic to that line of thinking, and if you and others feel strongly that we need to explicitly demarcate all of the role variables, maybe we could at least make it a bit cleaner here with a different name like \"prepare_workspace_root\".  It\u0027s also worth keeping in mind that we can write:\n  roles:\n    - role: prepare-workspace\n      root: {{zuul_workspace_root}\nif we wanted to shorten it further.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"78d65ca78487612ef6b64a95315a4b39860eb0ec","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prepare_workspace_workspace_root: /home/zuul/workspace"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_9bba2d6d","line":2,"updated":"2017-03-20 23:45:03.000000000","message":"My personal feeling is that we should follow the practices of the Ansible community. I\u0027ve not looked outside the walls of Ursula much, but it seems to me that a global zuul_workspace variable is fine for zuul\u0027s own roles, and then anything else should accept paths as role parameters.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":4162,"name":"Paul Belanger","email":"pabelanger@redhat.com","username":"pabelanger"},"change_message_id":"84fe7ed86e99a849d156ba860f6013dbfb792e0b","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prepare_workspace_workspace_root: /home/zuul/workspace"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_e77fe363","line":2,"updated":"2017-03-21 16:08:12.000000000","message":"Right, workspace is really our base directory jobs are configured to use. In this case, it is /home/zuul.  We would make this prepare_workspace_rootdir/basedir/workdir moving forward.  The good news it looks like, the people do follow \u003crole name\u003e_\u003clocal var\u003e syntax.\n\nI also like the idea that jeblair mentioned about using role paramaters to be more specific in what variables we are passing.\n\nFor now, to keep things moving foward. zuul_workspace_root (global var) seems to be fine with everybody, so I\u0027ve uploaded PS9 to do that.\n\nI would like to establish some sort of style guide for zuul stdlib (even openstack-infra), maybe the next step forward is a spec / doc where we can document some of these items.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"51c34ae221067e1a0a6431a68a3585e79fd02ea0","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prepare_workspace_workspace_root: /home/zuul/workspace"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_d3404e59","line":2,"updated":"2017-03-17 22:17:36.000000000","message":"Why not just use \"zuul_workspace_root\" here?  That seems sufficiently unique considering the expected scope of this role.  Reduces some extra plumbing and avoids an extra-long variable name.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":6589,"name":"Jesse Keating","email":"jkeating@j2solutions.net","username":"jesse-keating"},"change_message_id":"58d3b1c6ced0555e790493b958600e7057fd0c4e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prepare_workspace_workspace_root: /home/zuul/workspace"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_56beccd4","line":2,"in_reply_to":"1a1ced50_9bba2d6d","updated":"2017-03-21 00:17:19.000000000","message":"I don\u0027t have a strong opinion on whether or not to namespace the variables. I guess it depends on who the intended re-users of the roles. If they\u0027re only being used by zuul developers to test zuul itself, then we could easily define a global set of variables to use and depend on. If these roles are going to be usable outside of zuul, by maybe consumers of zuul, then it would be more appropriate to namespace, as we do not know how end users will combine these roles, and staking claim on global namespaces would be rude.\n\nAll that said, the awkwardness here is \"workspace_workspace\", which is what caught my eye immediately. I do agree renaming the role could help with this, such as \"workspace_bootstrap\" as the role name. However looking at the role itself, it\u0027s doing more than just ensuring that the workspace directory exists, it\u0027s also launching a daemon, so maybe the definition of \"workspace\" means the entire environment in which work happens, and then we have a \"workdir\" which is the directory within the workspace that is used to stage files and such. So, if we used \"workdir_root\" instead of \"workspace_root\", (or just simply \"workdir\" we could leave the role name as it is, and then this variable comes \"prepare_workspace_workdir_root\" (or \"prepare_workspace_workdir\", which avoids the awkward name repetition.\n\nAm I correct that \"workspace\" refers more to the greater environment, and less than the specific directory path?","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":7191,"name":"Jamie Lennox","email":"jamielennox@gmail.com","username":"jamielennox"},"change_message_id":"e53c66835462f755e39e6a4a3320e30807d4b2c4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prepare_workspace_workspace_root: /home/zuul/workspace"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_50bed4d4","line":2,"in_reply_to":"1a1ced50_ae444974","updated":"2017-03-20 22:45:10.000000000","message":"I feel like i\u0027m agreeing with both of you.\n\nI always try and write role variables as \u003crole_name\u003e_\u003cvariable_name\u003e as I find it is at least keeps the variable namespace somewhat isolated. \n\nWhenever there are variables that apply to multiple roles i have a \u003cglobal_name\u003e_\u003cvariable_name\u003e that i then patch through to the role. Depending on how you re-use the role it can be done from the playback or just a big vars file, personally i prefer it in the playbook.\n\nI prefer this split because i think it makes the roles reusable. The question here is whether reusability is something we care about given how tied together all these roles are and that we want to provide users with only a global set of variables to care about.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":4162,"name":"Paul Belanger","email":"pabelanger@redhat.com","username":"pabelanger"},"change_message_id":"0f0d9692ee66bd2fe713c9c0b63c9922e46c8434","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prepare_workspace_workspace_root: /home/zuul/workspace"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_6e1251b6","line":2,"in_reply_to":"1a1ced50_d3404e59","updated":"2017-03-17 22:41:18.000000000","message":"Let me preface, as this is completely personal opinion and something I\u0027ve wanted to at-least get into some sort of design document for playbook / roles.\n\nI have a strong desire to implement / encore some sort of variable scoping for our roles.  Specifically, a role should only access local variables itself.  I know there is nothing stopping you to do this in ansible today (and more something specific to how puppet does variables) but I see it as a good way to implement some sort of interface for roles. It also should help protect us from spaghetti dependencies with roles.\n\nFor me, I\u0027ve been writing role variables with the following in mind, \u003crole name\u003e_\u003cvariable name\u003e. I the case here, prepare_workspace is the role, workspace_root the local variable.  This was part of my motivation at rename the role to bootstrap, so it didn\u0027t look weird.\n\nAdditionally, this patchset is a little bad because it doesn\u0027t completely follow what I just typed in the run-cover role, since it is accessing zuul_workspace_root directly.\n\nI can hopefully better demonstrate it with some existing code:\n\n  http://git.openstack.org/cgit/openstack/ansible-role-shade/tree/defaults/main.yaml\n  http://git.openstack.org/cgit/openstack/windmill/tree/playbooks/group_vars/nodepool-launcher.yaml\n\nPerhaps I am being a little paranoid, or making additional work, but I am concerned about how easier it is to group everything into a global vars.  Especially if every get to the point of separate repos for ansible roles.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"}],"playbooks/roles/run-bindep/tasks/main.yaml":[{"author":{"_account_id":4162,"name":"Paul Belanger","email":"pabelanger@redhat.com","username":"pabelanger"},"change_message_id":"84fe7ed86e99a849d156ba860f6013dbfb792e0b","unresolved":false,"context_lines":[{"line_number":2,"context_line":"- name: Run install-distro-packages.sh"},{"line_number":3,"context_line":"  shell: /usr/local/jenkins/slave_scripts/install-distro-packages.sh"},{"line_number":4,"context_line":"  args:"},{"line_number":5,"context_line":"    chdir: \"{{ zuul_workspace_root }}/src/{{ zuul.project }}\""}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_07995fc4","line":5,"updated":"2017-03-21 16:08:12.000000000","message":"Agreed, I never got around to updating this.  But now it is consider with above.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"},{"author":{"_account_id":6589,"name":"Jesse Keating","email":"jkeating@j2solutions.net","username":"jesse-keating"},"change_message_id":"58d3b1c6ced0555e790493b958600e7057fd0c4e","unresolved":false,"context_lines":[{"line_number":2,"context_line":"- name: Run install-distro-packages.sh"},{"line_number":3,"context_line":"  shell: /usr/local/jenkins/slave_scripts/install-distro-packages.sh"},{"line_number":4,"context_line":"  args:"},{"line_number":5,"context_line":"    chdir: \"{{ zuul_workspace_root }}/src/{{ zuul.project }}\""}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1a1ced50_d678fc2d","line":5,"updated":"2017-03-21 00:17:19.000000000","message":"As you\u0027ve pointed out, you tried to make prepare-workspace have a scoped variable, but now you\u0027re using the global variable here. It would make more sense if you stayed consistent and either scoped the global variable into role specific ones at the play level (and thus not depend on a site-specific global variable in a role) or give up that effort and simply rely on the global variable name throughout all these roles.\n\nSame comment goes to the other roles touched by this change.","commit_id":"c07eb5e9c907d7e6566e15e5330a08b44cd58bda"}]}