-
-
Notifications
You must be signed in to change notification settings - Fork 606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create project_shared_children files if they do not exist #706
Conversation
Thank you @swalkinshaw for pointing out that the symlink creation will fail if the parent directory of I also added the I also added a comment in
I also made it explicit that the |
|
||
- name: Create shared symlinks | ||
file: | ||
path: "{{ deploy_helper.new_release_path }}/{{ item.path }}" | ||
path: "{{ deploy_helper.new_release_path }}/{{ item.path | default('') }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we enforce the path
here? Items set to absent
aren't included here anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The absent
option allows an easy way for someone to remove something from /shared
if it will no longer be used in the next deployment(s). Although it is not necessary to remove items, and although people could remove them manually, facilitating this absent
option requires no extra tasks, so I thought it was convenient.
However, the absent
option does require the the two tasks you've marked to include when
conditions and item.path | default('')
. Yes these two tasks will skip for items set to absent
, but Ansible still attempts to template all the variables in the task. So, without a default for item.path
, the task would fail when people omit path
for items set to absent
.
If we keep this absent
option, I would like people to be able to omit path
because an item to be removed from /shared
will have no path
in the release directory, so the path
parameter doesn't make sense to include.
with_items: "{{ project_shared_children }}" | ||
|
||
- name: Ensure shared paths are absent | ||
file: | ||
path: "{{ deploy_helper.new_release_path }}/{{ item.path }}" | ||
path: "{{ deploy_helper.new_release_path }}/{{ item.path | default('') }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with here
Latest commit
These changes favor "more tasks that are simple" over "fewer tasks that are complex." I routinely have to fight my inclination toward the latter. If/when merged, probably ought to be a "squash and merge." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 squash, changelog, merge :)
edbd95b
to
720e2f2
Compare
Fixes this discourse thread.
The "Ensure shared sources are present" task uses the file module to create directories or files in the
shared
directory.The comments for
project_shared_children
in the deploy/defaults indicates that items in the list "are created if they don't exist." The problem is that if a user specifiestype: file
, this is fed to thestate
option for the file module, but whenstate
is "file
, the file will NOT be created if it does not exist."In addition, the file module will not create the subdirectories a file may be in, if they are missing.
This PR splits the "Ensure shared sources are present" task in two. The first task creates all the shared directories, including the parent directories of shared files. The second task touches the files.
Touch is necessary if we actually want to create files, but unfortunately it is never idempotent.
This approach should still allow for
type
/state
values such asabsent
.