Feat(Block on missing config) #42
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alithethird
changed the title
alithethird
commented
Feb 11, 2025
alithethird
requested review from
jdkandersson
and removed request for
nrobinaubertin and
weiiwang01
alithethird
commented
Feb 11, 2025
weiiwang01
approved these changes
Feb 11, 2025
Contributor
|
Is this fully ready? At the interface PR stage, this should be marked as draft please |
alithethird
commented
Feb 12, 2025
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
license-eye has checked 283 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 155 | 1 | 127 | 0 |
Click to see the invalid file list
- tests/unit/general/conftest.py
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
alithethird
commented
Feb 19, 2025
alithethird
commented
Feb 19, 2025
alithethird
changed the title
weiiwang01
approved these changes
Feb 24, 2025
jdkandersson
approved these changes
Feb 24, 2025
Contributor
jdkandersson
left a comment
jdkandersson
left a comment
There was a problem hiding this comment.
Please remember to update the charmcraft docs
github-merge-queue bot
pushed a commit
to canonical/charmcraft
that referenced
this pull request
Feb 26, 2025
This PR is related to 12 Factor [Block on missing required config PR](canonical/paas-charm#42) ## Rationale Some applications require certain config options to run and would crash if these options are not provided. To prevent this, we need to add an optional field to the config options and block the charm if a config option marked as non-optional has not been provided with a message that explains which config options are missing. To stay backward compatible the default value for the optional field will be true. Since it doesn’t make sense for config options that are non-optional to have a default value, charmcraft will look for this condition at pack time and error out. ## Extension Changes Adds a check for non-optional options with default values and throws `ExtensionError` when found. Adds documentation about the non-optional options. Reviewers: @jdkandersson @javierdelapuente @erinecon
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Some applications require certain configuration options to function correctly. With this PR when a non-optional config option is missing, the charm will go into the blocked state with a message telling which config options are missing.
Rationale
Some applications require certain config options to run and would crash if these options are not provided. To prevent this, we need to add an optional field to the config options and block the charm if a config option marked as non-optional has not been provided with a message that explains which config options are missing.
To stay backward compatible the default value for the optional field will be true.
Since it doesn’t make sense for config options that are non-optional to have a default value, charmcraft will look for this condition at pack time and error out.
Module Changes
2 new functions in
paas_charm.charm_state.pyare added to create theAppConfigclass from the list of configuration options:app_config_factory_create_configuration_attributepaas_charm.utils.build_validation_error_messagefunction is changed. Now it returns a tuple of 2 strings.The tuple consists of:
messages. Best fit for printing error logs.
A new function in
paas_charm.utils.py:is_user_defined_config: This function checks whether a config option is user-defined.Add a new folder to unit tests to check functions not affected by the framework.
Add a new folder to integration tests to check functionality in every framework by parametrizing the framework. This way we only need to write 1 function to test functionality in every framework.
Checklist
src-docsurgent,trivial,complex)