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 cron.py #40003

Closed
wants to merge 3 commits into from
Closed

Update cron.py #40003

wants to merge 3 commits into from

Conversation

Loreton
Copy link

@Loreton Loreton commented May 11, 2018

[Please refer to PR #39954]

If the line that we need to insert already exists but the number of BLANKs that separate the fields is different from the new line, a duplicated line will be inserted.

SUMMARY

Try to find out if a row already exists ignoring the number of BLANKs that separate the individual fields. If the entry already exists, the job name will be inserted.
To do that extra BLANKs inside new_line and old_line are removed before comparing them.
I tested it with a classical entry and with a special_time "@reboot" entry.

ISSUE TYPE
  • Feature Pull Request
  • New Module Pull Request
  • Bugfix Pull Request
  • Docs Pull Request
COMPONENT NAME

cron.py

ANSIBLE VERSION
ansible 2.5.0
  config file = /home/pi/GIT-REPO/Ansible/PiAnsible/playbooks/ansible.cfg
  configured module search path = [u'/home/pi/GIT-REPO/Ansible/PiAnsible/library']
  ansible python module location = /usr/local/lib/python2.7/dist-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.3 (default, Nov 24 2017, 21:13:24) [GCC 4.6.3]
ADDITIONAL INFORMATION

************************ using CURRENT module ************************:
--- crontab before  run ---
05     22  *     *     *      sudo shutdown -h now


--- crontab after run ---
05     22  *     *     *      sudo shutdown -h now
#Ansible: shutdown
05 22 * * * sudo shutdown -h now

************************ using NEW module ************************
--- crontab before  run ---
05     22  *     *     *      sudo shutdown -h now

--- crontab after run ---
#Ansible: shutdown
05 22 * * * sudo shutdown -h now

If the line that we need to insert already exists but the number of BLANKs that separate the fields is different from the new line, a duplicated line will be inserted.
@ansibot
Copy link
Contributor

ansibot commented May 11, 2018

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. 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. support:community This issue/PR relates to code supported by the Ansible community. labels May 11, 2018
@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label May 11, 2018
@ansibot
Copy link
Contributor

ansibot commented May 11, 2018

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

lib/ansible/modules/system/cron.py:393:43: multiple-statements More than one statement on a single line

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

lib/ansible/modules/system/cron.py:393:42: E701 multiple statements on one line (colon)
lib/ansible/modules/system/cron.py:395:34: E225 missing whitespace around operator
lib/ansible/modules/system/cron.py:395:54: E261 at least two spaces before inline comment
lib/ansible/modules/system/cron.py:396:59: E261 at least two spaces before inline comment
lib/ansible/modules/system/cron.py:396:60: E262 inline comment should start with '# '

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. and removed community_review In order to be merged, this PR must follow the community review workflow. labels May 11, 2018
modified for PEP 8 style guideline compliance
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label May 11, 2018
@ansibot
Copy link
Contributor

ansibot commented May 11, 2018

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

lib/ansible/modules/system/cron.py:393:0: trailing-whitespace Trailing whitespace
lib/ansible/modules/system/cron.py:396:0: trailing-whitespace Trailing whitespace

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

lib/ansible/modules/system/cron.py:393:43: W291 trailing whitespace
lib/ansible/modules/system/cron.py:396:59: W291 trailing whitespace

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label May 11, 2018
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and 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 14, 2018
@ansibot ansibot added affects_2.6 This issue/PR affects Ansible v2.6 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 May 22, 2018
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed community_review In order to be merged, this PR must follow the community review workflow. support:community This issue/PR relates to code supported by the Ansible community. labels Sep 19, 2018
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 5, 2018
@ansibot ansibot added needs_maintainer Ansibot is unable to identify maintainers for this PR. (Check `author` in docs or BOTMETA.yml) support:community This issue/PR relates to code supported by the Ansible community. labels Oct 5, 2018
@ansibot ansibot removed the needs_maintainer Ansibot is unable to identify maintainers for this PR. (Check `author` in docs or BOTMETA.yml) label Nov 9, 2018
@ansibot
Copy link
Contributor

ansibot commented Dec 12, 2018

@ansibot ansibot added the system System category label Feb 17, 2019
@ansibot ansibot added the needs_repo This PR no longer has an associated branch as it was removed by the submitter. label Mar 13, 2019
@ansibot
Copy link
Contributor

ansibot commented Jul 31, 2019

@Loreton Your branch does not contain a shippable.yml file. Please rebase your branch to trigger running of current tests.

click here for bot help

@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_shippable This PR lacks a shippable.yml in its tree. Please rebase your branch to include the latest CI tests. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Jul 31, 2019
@ansibot
Copy link
Contributor

ansibot commented Jul 31, 2019

@Loreton Your branch does not contain a shippable.yml file. Please rebase your branch to trigger running of current tests.

click here for bot help

@gundalow
Copy link
Contributor

gundalow commented Sep 3, 2019

@Loreton Will you be able to still work on this PR?
needs_info

@gundalow gundalow added pr_day Has been reviewed during a PR review Day candidate_to_close Think we can close this, though need to check with Core labels Sep 3, 2019
@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label Sep 3, 2019
@Loreton
Copy link
Author

Loreton commented Sep 4, 2019 via email

@gundalow
Copy link
Contributor

@Loreton Thank you for your comment. I think we'd need tests for this, which I understand you don't have time for at the moment, which is OK.

Therefore I'll close this PR.

A new PR in the future that includes integrations tests would be welcome.

@gundalow gundalow closed this Sep 19, 2019
@ansible ansible locked and limited conversation to collaborators Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 candidate_to_close Think we can close this, though need to check with Core feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_info This issue requires further information. Please answer any outstanding questions. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_repo This PR no longer has an associated branch as it was removed by the submitter. needs_shippable This PR lacks a shippable.yml in its tree. Please rebase your branch to include the latest CI tests. new_contributor This PR is the first contribution by a new community member. pr_day Has been reviewed during a PR review Day stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. system System category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants