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

module_utils: Consolidate set_owner_if_different & set_group_if_different #75742

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

moreati
Copy link
Contributor

@moreati moreati commented Sep 21, 2021

SUMMARY

This merges implementations of AnsibleModule.set_owner_if_different() & AnsibleModule.set_group_if_different() into a single method. The benefits are

  • reduction in code duplication
  • makes possible halving the number of calls to os.lchown() when the copy module sets both owner and group.

There should be no outward change in behavior. The two original methods become thin wrappers around the new one.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

This PR is related to #75741. If both are merged, then the second benefit above can be realised - by replacing calls in copy.chown_recursive().

@ansibot ansibot added affects_2.12 core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 21, 2021
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Sep 21, 2021
@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 Sep 29, 2021
@moreati moreati closed this Nov 12, 2021
@moreati moreati reopened this Nov 12, 2021
@ansibot ansibot removed 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 12, 2021
@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 24, 2021
@mattclay mattclay added the P3 Priority 3 - Approved, No Time Limitation label Jul 13, 2022
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. core_review In order to be merged, this PR must follow the core review workflow. labels Jul 29, 2022
@moreati
Copy link
Contributor Author

moreati commented Jul 31, 2022

/rebuild

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 31, 2022
@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 Aug 8, 2022
…rent

This merges implementations of `AnsibleModule.set_owner_if_different()`
& `AnsibleModule.set_group_if_different()` into a single method. The
benefits are
- reduction in code duplication
- makes possible halving the number of calls to `os.lchown()` when the
  copy module sets both owner and group.

There should be no outward change in behavior. The two original methods
become thin wrappers around the new one.

This PR is related to ansible#75741. If both are merged, then the second
benefit above can be realised - by replacing calls in
`copy.chown_recursive()`.
@ansibot ansibot removed 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 Jul 8, 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 Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.12 core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. P3 Priority 3 - Approved, No Time Limitation 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:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants