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

Set feature flag base on setting #15808

Merged

Conversation

TheRealHaoLiu
Copy link
Member

SUMMARY

Set value of feature flag base on local setting

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API
AWX VERSION

ADDITIONAL INFORMATION

@TheRealHaoLiu TheRealHaoLiu marked this pull request as ready for review February 3, 2025 20:46
@AlanCoding
Copy link
Member

Context:

This would address my comments on #15799

@AlanCoding
Copy link
Member

More context, people have talked this out at some point or another:

#15702 would add dynaconf. The key feature of that is getting settings from environment variables. This includes nested variables, like FLAGS__FEATURE_INDIRECT_NODE_COUNTING_ENABLED__0__value=true. Some syntax like that would turn on indirect node counting.

But that's unlikely to be ready very soon, and we want to unblock the work that this would allow.

This will still probably change a little bit what that PR is doing so I want to get everyone in the loop and then we can go ahead with this merge.

Copy link

codecov bot commented Feb 3, 2025

Codecov Report

Attention: Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 75.28%. Comparing base (a74e730) to head (d7d7a8e).
Report is 1 commits behind head on devel.

✅ All tests successful. No failed tests found.

Copy link
Contributor

@pb82 pb82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlanCoding
Copy link
Member

This topic keeps getting deeper

Apparently, the FLAGS__FEATURE_INDIRECT_NODE_COUNTING_ENABLED__0__value syntax is a recent addition - CITATION NEEDED

dynaconf/dynaconf#1001

That appears to maybe be when this was added. Merged Apr 19, 2024. Check commit, not in any tags. Waiting for the next major release apparently.

@TheRealHaoLiu your PR now actually makes sense. Without this piece of the puzzle, I have to admit this appeared to be nonsense. Why not just set the nested value? It seems we have a long way to go before dynaconf will allow an environment variable to do this.

@TheRealHaoLiu TheRealHaoLiu force-pushed the set-feature-flag-base-on-settings branch from 46c4bbf to d7d7a8e Compare February 4, 2025 15:41
Copy link

sonarqubecloud bot commented Feb 4, 2025

@TheRealHaoLiu TheRealHaoLiu merged commit 15932e3 into ansible:devel Feb 4, 2025
23 of 26 checks passed
@TheRealHaoLiu TheRealHaoLiu deleted the set-feature-flag-base-on-settings branch February 4, 2025 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants