Skip to content

Configuration arguments validation#258

Closed
jeremydvoss wants to merge 3 commits intomicrosoft:mainfrom
jeremydvoss:type-validation
Closed

Configuration arguments validation#258
jeremydvoss wants to merge 3 commits intomicrosoft:mainfrom
jeremydvoss:type-validation

Conversation

@jeremydvoss
Copy link
Copy Markdown
Member

@jeremydvoss jeremydvoss commented Mar 14, 2023

Replacing with #262 262

@jeremydvoss jeremydvoss force-pushed the type-validation branch 2 times, most recently from 5702e89 to 85243fa Compare March 14, 2023 21:38
@jeremydvoss jeremydvoss marked this pull request as ready for review March 14, 2023 21:39
@jeremydvoss jeremydvoss requested review from a team and lzchen as code owners March 14, 2023 21:39
@jeremydvoss jeremydvoss linked an issue Mar 14, 2023 that may be closed by this pull request
assert _is_instance_or_none(
configurations.get("exclude_instrumentations"), Sequence
)
assert _is_instance_or_none(configurations.get("resource"), Resource)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The idea of this PR is not really to do type checking (it would be too cumbersome to always check for typing, plus this is why we use typing.

We want to only have a few validation checks for logic (like interval needs to be > 0).

The idea for Python and for the distro is that if input is not the correct type, it will not do anything with that input.

assert _is_instance_or_none(configurations.get("logging_level"), int)
assert _is_instance_or_none(configurations.get("logger_name"), str)
assert _is_instance_or_none(
configurations.get("logging_export_interval_millis"), int
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-negative values for intervals.

)
for library in _SUPPORTED_INSTRUMENTED_LIBRARIES:
assert _is_instance_or_none(
configurations.get(library + "_config"), Dict
Copy link
Copy Markdown
Contributor

@lzchen lzchen Mar 20, 2023

Choose a reason for hiding this comment

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

Might have to rebase this and validate instrumentation_config

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add validation for configuration options

2 participants