Skip to content

make constant loading dynamic - #81947

Draft
bcoca wants to merge 8 commits into
ansible:develfrom
bcoca:dynamic_constant
Draft

make constant loading dynamic#81947
bcoca wants to merge 8 commits into
ansible:develfrom
bcoca:dynamic_constant

Conversation

@bcoca

@bcoca bcoca commented Oct 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Don't load a configuration unless we need it.

Config deprecations/warnings less noisy as they will only trigger for config sources if setting is consumed

ISSUE TYPE
  • Feature Pull Request

@ansibot ansibot added feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 10, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Oct 11, 2023
@ansible ansible deleted a comment from ansibot Oct 13, 2023
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Oct 13, 2023
@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Oct 17, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Oct 18, 2023
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Oct 19, 2023
webknjaz
webknjaz reviewed Oct 31, 2023
View reviewed changes
Comment thread lib/ansible/constants.py Outdated
webknjaz
webknjaz reviewed Oct 31, 2023
View reviewed changes
Comment thread lib/ansible/constants.py
@webknjaz

webknjaz commented Oct 31, 2023

Copy link
Copy Markdown
Member

This somehow makes the fork_safe_stdio integration test fail.

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Oct 31, 2023
@bcoca

bcoca commented Oct 31, 2023

Copy link
Copy Markdown
Member Author

This somehow makes the fork_safe_stdio integration test fail.

Well, since this can dynamically create a constant post fork, it is a trade off between generating all constants up front even when we don't need most and recreating a constant we use a lot but only post fork. These constants are basically module level variables we inject. I can see how that can create issues, specially since display uses the config system.

@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Oct 31, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Nov 1, 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 Nov 15, 2023
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jul 2, 2024
@ansibot ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Nov 7, 2024
@bcoca bcoca mentioned this pull request Nov 7, 2024
Closed
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. 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 7, 2024
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Nov 8, 2024
@ansible ansible deleted a comment from ansibot Nov 8, 2024
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Nov 8, 2024
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Nov 12, 2024
Akasurde
Akasurde reviewed Nov 13, 2024
View reviewed changes
Comment thread lib/ansible/constants.py Outdated
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Nov 14, 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 Nov 28, 2024
@bcoca

bcoca commented Nov 29, 2024

Copy link
Copy Markdown
Member Author

@webknjaz possibly, in my tests the startup is faster, since it only loads what it needs, but if you end up needing most settings it is slower in the end, why i added a forced dump method for ansible-config which is basically how it works before the patch. I'll check the stdin test to see what is going on.

@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. 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 Dec 2, 2024
@webknjaz

webknjaz commented Jan 21, 2025

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

azure-pipelines Bot commented Jan 21, 2025

Copy link
Copy Markdown
Pipelines were unable to run due to time out waiting for the pull request to finish merging.

@webknjaz

webknjaz commented Jan 21, 2025

Copy link
Copy Markdown
Member

/azp run

@webknjaz webknjaz removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 21, 2025
@azure-pipelines

azure-pipelines Bot commented Jan 21, 2025

Copy link
Copy Markdown
Pipelines were unable to run due to time out waiting for the pull request to finish merging.

@webknjaz

webknjaz commented Jan 22, 2025

Copy link
Copy Markdown
Member

@bcoca another unimportant_ci candidate?

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Jan 23, 2025
@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 Feb 4, 2025
bcoca and others added 8 commits February 4, 2025 14:47
@bcoca
make constant loading dynamic
b191541 
now constants get populated 'on demand' and cached once set
We do need to ensure we initalize deps (constants use in templating for other constants)
@bcoca
comments
e81591b
@bcoca
avoid dynamic constants for config dumps
1b74009
@bcoca
requteo
8db88d2
@bcoca
no dup
bd48a11
@bcoca
fixed test to handle new behaviour
5f6eb47 
deprecations for setting sources only appear when settings is used
@bcoca
clarified comments
5c09c55
@Akasurde @bcoca
Review requests
47371a8 
Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. 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 Feb 4, 2025
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Feb 5, 2025
@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 Feb 19, 2025
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label May 6, 2025
@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@webknjaz webknjaz webknjaz left review comments
@Akasurde Akasurde Akasurde left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

ci_verified Changes made in this PR are causing tests to fail. feature This issue/PR relates to a feature request. 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_pr This PR has not been pushed to for more than one year.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bcoca @webknjaz @Akasurde @sivel @ansibot