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

Refactor loading of vars_files to simplify and properly implement expectations #80505

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

Conversation

sivel
Copy link
Member

@sivel sivel commented Apr 12, 2023

SUMMARY

Refactor loading of vars_files to simplify and properly implement expectations. Fixes #80483

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/vars/manager.py

ADDITIONAL INFORMATION

Reviewers will almost certainly want to view the changes ignoring whitespace: https://github.com/ansible/ansible/pull/80505/files?w=1

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.16 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. labels Apr 12, 2023
@bcoca
Copy link
Member

bcoca commented Apr 12, 2023

A few things i've been thinking for this feature:

  • cache the file paths read when it was called w/o host (reset cache per play)
  • add deprecation message if any files read with host are not in previous cache
  • also add deprecation message if the vars_file_item is a list and point to first_found filter as alternative
  • add deprecation to 'skip message' when there is undefined var and no 'host'

Once deprecation periods are over this becomes a very simple feature we should only call 1/play

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Apr 13, 2023
@sivel sivel marked this pull request as ready for review April 13, 2023 19:42
@ansibot ansibot removed the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Apr 13, 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 Apr 25, 2023
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 11, 2023
@ansibot ansibot added has_issue and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 12, 2023
test/integration/targets/vars_files/runme.sh Outdated Show resolved Hide resolved
test/integration/targets/vars_files/runme.sh Outdated Show resolved Hide resolved
@ansibot ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Aug 3, 2023
@ansibot ansibot 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 Aug 3, 2023
@nitzmahone nitzmahone self-requested a review August 16, 2023 15:56
@sivel sivel requested a review from bcoca August 16, 2023 15:57
@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 16, 2023
@ansibot ansibot removed the stale_review Updates were made after the last review and the last review is more than 7 days old. label Aug 16, 2023
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Mar 19, 2024
@ansibot ansibot added stale_review Updates were made after the last review and the last review is more than 7 days old. 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 Apr 17, 2024
@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 1, 2024
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.16 bug This issue/PR relates to a bug. data_tagging has_issue 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. stale_review Updates were made after the last review and the last review is more than 7 days old.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vars_files depending on host vars no longer fails as of 2.15 with error_on_undefined_vars disabled
4 participants