ini_file: Don't creates new file instead of following symlink #6546
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a bug fix that address a situation where `community.general.ini_file` was destroying symlinks instead of updating of updating their targets. Closes: ansible-collections#6470
ansibullbot
added
bug
There was a problem hiding this comment.
Thanks for your contribution!
I'm wondering whether changing the behavior is a good idea, since some folks might rely on it. I think it might be better to make this behavior configurable, by adding a follow option (similar to https://docs.ansible.com/ansible/devel/collections/ansible/builtin/copy_module.html#parameter-follow).
| @@ -0,0 +1,3 @@ | |||
| --- | |||
| bugfixes: | |||
| - "ini_file - Follow the symlinks instead of replacing them (https://github.com/ansible-collections/community.general/issues/6470)." | |||
There was a problem hiding this comment.
Suggested change
| - "ini_file - Follow the symlinks instead of replacing them (https://github.com/ansible-collections/community.general/issues/6470)." | |
| - "ini_file - follow the symlinks instead of replacing them (https://github.com/ansible-collections/community.general/issues/6470, https://github.com/ansible-collections/community.general/pull/6546)." |
felixfontein
added
check-before-release
|
Hi @felixfontein! From my view point, this was more an unpredictable bug. But I understand your point of view. This is a bit of a XKCD 1172: Workflow situation (https://www.explainxkcd.com/wiki/index.php/1172:_Workflow). I'm a bit hesitant to add a key just for the users who were relying on the previous behaviour. Would you be fine with an entry in the documentation that cover the symlink case? |
|
@goneri I see the current behavior as somewhat surprising (since most modules don't act the same), but it does have a certain consistency. I can imagine some use-cases where the current behavior is explicitly needed and changing it would be a bug. So from this POV this is a breaking change and cannot happen without a longer deprecation period. In any case, before it's decided whether the default behavior should change, it makes sense to make the behavior configurable, since both behaviors make sense for specific use-cases. |
|
Agreed. |
If `poth` points on a symlink and `follow` is true, the `ini_file` plugin will preserve the symlink and modify the target file.
- yes/no -> true/false. - new key will be introduced in 7.1.0. - clean up the `state=link` part.
ansibullbot
added
needs_revision
felixfontein
approved these changes
May 26, 2023
felixfontein
removed
the
check-before-release
Backport to stable-7: 💚 backport PR created✅ Backport PR branch: Backported as #6598 🤖 @patchback |
|
@goneri thanks for your contribution! |
patchback bot
pushed a commit
that referenced
this pull request
May 29, 2023
* ini_file: Don't creates new file instead of following symlink This is a bug fix that address a situation where `community.general.ini_file` was destroying symlinks instead of updating of updating their targets. Closes: #6470 * ini_file: add the follow parameter If `poth` points on a symlink and `follow` is true, the `ini_file` plugin will preserve the symlink and modify the target file. * adjust the documentation of the new key - yes/no -> true/false. - new key will be introduced in 7.1.0. - clean up the `state=link` part. (cherry picked from commit c76af60)
felixfontein
pushed a commit
that referenced
this pull request
May 29, 2023
…le instead of following symlink (#6598) ini_file: Don't creates new file instead of following symlink (#6546) * ini_file: Don't creates new file instead of following symlink This is a bug fix that address a situation where `community.general.ini_file` was destroying symlinks instead of updating of updating their targets. Closes: #6470 * ini_file: add the follow parameter If `poth` points on a symlink and `follow` is true, the `ini_file` plugin will preserve the symlink and modify the target file. * adjust the documentation of the new key - yes/no -> true/false. - new key will be introduced in 7.1.0. - clean up the `state=link` part. (cherry picked from commit c76af60) Co-authored-by: Gonéri Le Bouder <[email protected]>
jikamens
added a commit
to jikamens/community.general
that referenced
this pull request
Sep 16, 2023
It appears that ansible-collections#6546 added symlink tests but neglected to add them to main.yml so they weren't being executed.
felixfontein
pushed a commit
that referenced
this pull request
Sep 17, 2023
…7273) * Add `ignore_spaces` option to `ini_file` to ignore spacing changes Add a new `ignore_spaces` option to the `ini_file` module which, if true, prevents the module from changing a line in a file if the only thing that would change by doing so is whitespace before or after the `=`. Also add test cases for this new functionality. There were previously no tests for `ini_file` at all, and this doesn't attempt to fix that, but it does add tests to ensure that the new behavior implemented here as well as the old behavior in the affected code are correct. Fixes #7202. * Add changelog fragment * pep8 / pylint * remove unused import * fix typo in comment in integration test file * Add symlink tests to main.yml It appears that #6546 added symlink tests but neglected to add them to main.yml so they weren't being executed. * ini_file symlink tests; create output files in correct location * Add integration tests for ini_file ignore_spaces * PR feedback
patchback bot
pushed a commit
that referenced
this pull request
Sep 17, 2023
…7273) * Add `ignore_spaces` option to `ini_file` to ignore spacing changes Add a new `ignore_spaces` option to the `ini_file` module which, if true, prevents the module from changing a line in a file if the only thing that would change by doing so is whitespace before or after the `=`. Also add test cases for this new functionality. There were previously no tests for `ini_file` at all, and this doesn't attempt to fix that, but it does add tests to ensure that the new behavior implemented here as well as the old behavior in the affected code are correct. Fixes #7202. * Add changelog fragment * pep8 / pylint * remove unused import * fix typo in comment in integration test file * Add symlink tests to main.yml It appears that #6546 added symlink tests but neglected to add them to main.yml so they weren't being executed. * ini_file symlink tests; create output files in correct location * Add integration tests for ini_file ignore_spaces * PR feedback (cherry picked from commit 8a9b982)
felixfontein
pushed a commit
that referenced
this pull request
Sep 17, 2023
…`ini_file` to ignore spacing changes (#7279) Add `ignore_spaces` option to `ini_file` to ignore spacing changes (#7273) * Add `ignore_spaces` option to `ini_file` to ignore spacing changes Add a new `ignore_spaces` option to the `ini_file` module which, if true, prevents the module from changing a line in a file if the only thing that would change by doing so is whitespace before or after the `=`. Also add test cases for this new functionality. There were previously no tests for `ini_file` at all, and this doesn't attempt to fix that, but it does add tests to ensure that the new behavior implemented here as well as the old behavior in the affected code are correct. Fixes #7202. * Add changelog fragment * pep8 / pylint * remove unused import * fix typo in comment in integration test file * Add symlink tests to main.yml It appears that #6546 added symlink tests but neglected to add them to main.yml so they weren't being executed. * ini_file symlink tests; create output files in correct location * Add integration tests for ini_file ignore_spaces * PR feedback (cherry picked from commit 8a9b982) Co-authored-by: Jonathan Kamens <[email protected]>
etrombly
pushed a commit
to etrombly/community.general
that referenced
this pull request
Oct 25, 2023
…nsible-collections#7273) * Add `ignore_spaces` option to `ini_file` to ignore spacing changes Add a new `ignore_spaces` option to the `ini_file` module which, if true, prevents the module from changing a line in a file if the only thing that would change by doing so is whitespace before or after the `=`. Also add test cases for this new functionality. There were previously no tests for `ini_file` at all, and this doesn't attempt to fix that, but it does add tests to ensure that the new behavior implemented here as well as the old behavior in the affected code are correct. Fixes ansible-collections#7202. * Add changelog fragment * pep8 / pylint * remove unused import * fix typo in comment in integration test file * Add symlink tests to main.yml It appears that ansible-collections#6546 added symlink tests but neglected to add them to main.yml so they weren't being executed. * ini_file symlink tests; create output files in correct location * Add integration tests for ini_file ignore_spaces * PR feedback
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
This is a bug fix that address a situation where
community.general.ini_filewas destroying symlinks instead of updating of updating their targets.If
pothpoints on a symlink andfollowis true, theini_fileplugin will preserve the symlink and modify the target file.Closes: #6470
ISSUE TYPE
COMPONENT NAME
ini_file