type coercion for configuration parameters #2878
Merged
Conversation
This file contains 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
An initial pass at reworking the processing of configuration parameters to address #2852. - nils to produce errors when user-supplied for params that don't support them - introduce a common shared type coercion layer for all config values so that things like environment variable and YAML values are both consistently coerced - boolean coercion is no longer special cased and handled by the coercion layer - use `#call` instead of `instance_eval` on procs (benchmark supported)
- fix `raw_value` -> `string_array` variable rename needed for one remaining instance - anticipate a default value being a proc and call it before yielding the value - explicitly check for `#nil?` instead of truthiness, as `false` is certainly a valid non-nil config value
for legacy reasons... 1. permit tests to specify invalid configuration 2. permit `manual_start` to specify invalid configuration 3. permit int values for float based params and vice versa 4. permit string values for symbol based params and vice versa and also... 5. fix calls to `evaluate_and_apply_transformations` that were broken when the method was updated to accept a config category argument
- treat :app_name as a special case. technically it should be classified as an `Array` type with a custom (semicolon) delimiter override but for backwards compatibility just make the transform dynamic - allow unit tests to do screwy things users shouldn't be permitted to do
* remove orphaned type map content from environment source * for bools convert symbols to strings before converting to true/false * rework unit tests to match lib changes
* revert from `call` back to `instance_eval` so that the original intended scope is preserved * update tests to match lib changes
trust the config manager tests to handle these situations
config manager test updates
float args for int params will be rounded, so use an int to help with setting accurate expectations
- don't bother with calculations if a callback isn't registered - assume the cached value has been evaluated already
* optimisation: don't bother with empty manual source opts hashes * restoration of legacy behavior: permit config params like :web_transactions_apdex to behave as expected (permit them to not have any entry in default source for things like a class type or a default value
* use `#[]` instead of `#fetch` on the @cache hash to light up its functionality properly * update default source test to perform a cache lookup call (previously a now-skipped callback call would have performed it)
rejecting empty hashes apparently wipes previously set values from non-empty hashes, so for now allow the empties to flow through as before
if the given value is already of the type the transform is expected to produce, then just let it pass through
update the test to expect a default value to be used
SimpleCov Report
|
fallwith
requested review from
hannahramadan,
kaylareopelle and
tannalynn
as code owners
tannalynn
approved these changes
Oct 17, 2024
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.
Rework the processing of configuration parameters to address #2852.
:transformentries#to_f) and Float values for Integer params (using#round):allow_nilsetting to enforce non-nil values for params that don't support nilresolves #2852