Skip to content
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

Update lineinfile.py: uniq option added #80678

Open
wants to merge 7 commits into
base: devel
Choose a base branch
from
Open

Conversation

mcgru
Copy link

@mcgru mcgru commented May 1, 2023

uniq option: if set to yes/true, leaves only one (first) LINE in file, removing all other same lines.

SUMMARY

In some cases duplicate lines appears in target 'path' file after applying wrong regexp-based lineinfile tasks.
This 'uniq' option removes all duplicates of 'line' in b_lines array before writing to the 'path'.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lineinfile

ADDITIONAL INFORMATION

'uniq' option added to DESCRIPTION, to the modules options, to the present()-function arguments.
Rewrites b_lines array if 'uniq' set to true, makes msg to 'made line unique' and 'changed' to true, right before check for module._diff .


uniq option: if set to yes/true, leaves only one (first) LINE in file, removing all other same lines.
@ansibot ansibot added affects_2.16 feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. labels May 1, 2023
@ansibot
Copy link
Contributor

ansibot commented May 1, 2023

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/lineinfile.py:0:0: option-incorrect-version-added: version_added for new option (uniq) should be '2.16'. Currently StrictVersion ('2.14')

The test ansible-test sanity --test pep8 [explain] failed with 9 errors:

lib/ansible/modules/lineinfile.py:327:7: E111: indentation is not a multiple of 4
lib/ansible/modules/lineinfile.py:327:11: E225: missing whitespace around operator
lib/ansible/modules/lineinfile.py:328:7: E111: indentation is not a multiple of 4
lib/ansible/modules/lineinfile.py:329:7: E111: indentation is not a multiple of 4
lib/ansible/modules/lineinfile.py:329:18: E201: whitespace after '['
lib/ansible/modules/lineinfile.py:329:73: E201: whitespace after '('
lib/ansible/modules/lineinfile.py:329:113: E202: whitespace before ')'
lib/ansible/modules/lineinfile.py:329:115: E202: whitespace before ']'
lib/ansible/modules/lineinfile.py:330:7: E111: indentation is not a multiple of 4

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/modules/lineinfile.py:329:40: simplifiable-condition: Boolean condition 'l.rstrip(b'\r\n') != b_line or l not in used and (used.add(l) or True)' may be simplified to 'l.rstrip(b'\r\n') != b_line or l not in used'
lib/ansible/modules/lineinfile.py:329:61: used-before-assignment: Using variable 'b_line' before assignment

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 1, 2023
@ansibot
Copy link
Contributor

ansibot commented May 1, 2023

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/lineinfile.py:0:0: option-incorrect-version-added: version_added for new option (uniq) should be '2.16'. Currently StrictVersion ('2.14')

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/lineinfile.py:496:41: simplifiable-condition: Boolean condition 'l.rstrip(b'\r\n') != b_line or l not in used and (used.add(l) or True)' may be simplified to 'l.rstrip(b'\r\n') != b_line or l not in used'

click here for bot help

@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 1, 2023
@ansibot
Copy link
Contributor

ansibot commented May 1, 2023

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/lineinfile.py:498:19: unneeded-not: Consider changing "not __is_absent.counter > 1" to "__is_absent.counter <= 1"

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 1, 2023
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 1, 2023
@bcoca
Copy link
Member

bcoca commented May 2, 2023

aside from missing tests and changelog, wouldn't the replace module do a better job?
needs_info

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label May 2, 2023
@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label May 2, 2023
@mcgru
Copy link
Author

mcgru commented May 3, 2023

on Changelog, i can see diff at https://github.com/ansible/ansible/pull/80678/files , can you?
shortly, i've tried to describe new 'uniq' option earlier as: it removes duplicate lines right after (during recovering) 'lineinfile'.
on tests absence, sorry, i'm new at github contribution, i saw that ansibot did some tests like 'ansible-test sanity --test validate-modules' and 'ansible-test sanity --test pep8'. But functionally, i did tests on my side only. Please, let me know how to pass it on github.com ?
on 'replace' module: usually, it replaces ALL lines matched, but in my case we need to remove all (errorneous) but one (true).
I suppose, that there is a decision with some non-trivial usage with {{item.index}} and/or 'after' option in 'lineinfile' or 'replace', but in my (not only single me who do want it, i found dosens of such requests) opinion, such option simplifies the recover of wrongly modified files after bad 'lineinfile' call.

Why i wanted this new feature: sometimes people use wrong regexp in lineinfile, that lead to warnings during 'verify' or 'restart notifiers' in most cases, leaving number of duplicate lines in target 'path' config file after number of lineinfile runs. But in some cases that duplicates could brake application. Of course, usage of 'uniq' could brake those configs where dups are intentionally used - thats why default is 'no' for 'uniq'

@ansibot ansibot removed the needs_info This issue requires further information. Please answer any outstanding questions. label May 3, 2023
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 11, 2023
@mcgru
Copy link
Author

mcgru commented Dec 14, 2023

Sorry for silence. Is there any obstacles that prevents include this PR into main branch?

@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jan 9, 2024
@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Nov 2, 2024
@ansibot
Copy link
Contributor

ansibot commented Nov 2, 2024

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/lineinfile.py:0:0: nonexistent-parameter-documented: Argument 'others' is listed in DOCUMENTATION.options, but not accepted by the module argument_spec
lib/ansible/modules/lineinfile.py:0:0: option-incorrect-version-added: version_added for new option (others) should be '2.19'. Currently StrictVersion ('0.0')
lib/ansible/modules/lineinfile.py:0:0: option-incorrect-version-added: version_added for new option (uniq) should be '2.19'. Currently StrictVersion ('2.16')

click here for bot help

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.16 ci_verified Changes made in this PR are causing tests to fail. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants