opentelemetry-configuration: resolve false-positive warning logs for newer schema minor version#5436
opentelemetry-configuration: resolve false-positive warning logs for newer schema minor version#5436herin049 wants to merge 4 commits into
Conversation
…newer minor version
herin049
changed the title
| if _SUPPORTED_SCHEMA_MAJOR > 0: | ||
| versions.append(f"{_SUPPORTED_SCHEMA_MAJOR - 1}.0") |
There was a problem hiding this comment.
I guess we can just hardcode 0.4 since we aren't going backwards from 1.0?
| # https://github.com/open-telemetry/opentelemetry-configuration/blob/main/VERSIONING.md | ||
| _SUPPORTED_SCHEMA_MAJOR = 1 | ||
| _SUPPORTED_SCHEMA_MINOR = 0 | ||
| _SUPPORTED_SCHEMA_MINOR = 1 |
There was a problem hiding this comment.
While the warning may be annoying the packages are shipping a 1.0 schema so if we bump here we are breaking support for them?
There was a problem hiding this comment.
_validate_file_format() doesn't log anything if the current minor is smaller than the latest supported. From my understanding, older minor versions should be fully supported, it's just newer supported minor versions that may include functionality which isn't implemented in the SDK (which is why we log). It's the same as SemVer semantics.
There was a problem hiding this comment.
Ofc, all bets are off when using developmental features.
There was a problem hiding this comment.
yeah. just tested. didn't break for me while still using 1.0
There was a problem hiding this comment.
Sorry I was confused by the reading the newer major test. Should we test the older minor as well if _SUPPORTED_SCHEMA_MINOR > 0?
xrmx
left a comment
xrmx
left a comment
There was a problem hiding this comment.
LGTM, a couple of nits in the tests
xrmx
moved this from Ready for review
to Approved PRs that need fixes
in Python PR digest
3 participants
Description
Fixes issue where a false-positive warning log is emitted while using the latest supported schema version 1.1.
Fixes #5435
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Does This PR Require a Contrib Repo Change?
Checklist: