Add filename option to apt repository add #368
Conversation
|
This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution! Please read this blog post to see the reasons why I mark pull requests as stale. |
|
Not stale |
|
This issue is no longer marked for closure. |
|
This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution! Please read this blog post to see the reasons why I mark pull requests as stale. |
|
not stale (unless superseded by #369) |
|
This issue is no longer marked for closure. |
README.md
Outdated
| @@ -56,6 +56,7 @@ The main Docker repo URL, common between Debian and RHEL systems. | |||
| docker_apt_repository: "deb [arch={{ docker_apt_arch }}] {{ docker_repo_url }}/{{ ansible_distribution | lower }} {{ ansible_distribution_release }} {{ docker_apt_release_channel }}" | |||
| docker_apt_ignore_key_error: True | |||
| docker_apt_gpg_key: "{{ docker_repo_url }}/{{ ansible_distribution | lower }}/gpg" | |||
| docker_apt_filename: omit | |||
There was a problem hiding this comment.
For this option, could you just put instead of omit, custom-filename-here-if-desired or something that would be a more practical example. Or maybe we could leave it empty in defaults/main.yml instead, that might be preferable (docker_apt_filename: '', and then document that value here, so it is more obvious it should be a string.
There was a problem hiding this comment.
Agree that the latter is probably preferable. Unfortunately, it seems like you can't give an empty string for that argument. This PR fail CI, but #369, which has a non-empty string, passes CI.
We could hack around it with something like filename: {{ docker_apt_filename if docker_apt_filename != "" else omit }}, assuming that works.
Alternatively, it may make more sense to just close this in favor of #369, if you plan on releasing 7.0.0 soon. This PR is mainly intended as a temporary fix until the defaults get updated anyway.
|
This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution! Please read this blog post to see the reasons why I mark pull requests as stale. |
|
not stale |
|
This issue is no longer marked for closure. |
|
@geerlingguy, @kaysond what if we add something like - name: Add Docker repository.
apt_repository:
repo: "{{ docker_apt_repository }}"
filename: "{{ docker_apt_filename | default(omit) }}"
state: present
update_cache: true
when: docker_add_repo | boolIt's backward compatible with the current code. For next major version we can set a name (maybe "docker") for the file, but meanwhile people can start changing it to the desired name. |
|
Superseded by #369 |
Addresses #335 and #101 (i.e. I'd say it's technically a work around as opposed to a solution)