-
Notifications
You must be signed in to change notification settings - Fork 562
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
Allow relaxed instantiation of exporter configuration #9854
Conversation
Fixes a bug where deserialization of exporter configuration was case sensitive, such that a property `NUMBEROFSHARDS` would not map to an instance field `numberOfShards`. This isn't perfect, as now if we have two fields with the same name but different cases, only one of them will be set, using the latest value in the map (where the order is undefined). It should solve the most common cases however.
Unit Test Results 794 files + 2 794 suites +2 1h 33m 3s ⏱️ - 2m 0s For more details on these failures, see this check. Results for commit 0fd1eeb. ± Comparison against base commit b270a21. ♻️ This comment has been updated with latest results. |
d043b70
to
0fd1eeb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, looks good 👍
🔧 Do we really want to close #7628 or is that an improvement we might want to do later?
I think we should close it for now. While it would be nice, the nicest would still be to not have exporters ;) |
There was a flaky test, but I highly doubt it has anything to do with this PR, since the test is about message correlation/snapshots, and not exporter configuration (if the config was broken, you would think it would fail consistently). bors merge |
Build succeeded: |
Backport failed for Please cherry-pick the changes locally. git fetch origin stable/1.3
git worktree add -d .worktree/backport-9854-to-stable/1.3 origin/stable/1.3
cd .worktree/backport-9854-to-stable/1.3
git checkout -b backport-9854-to-stable/1.3
ancref=$(git merge-base b270a213635b7f5dc2b1497c02acaec1057b149d 0fd1eebc8bcf07d248ecf628e19f70384a5dc1cb)
git cherry-pick -x $ancref..0fd1eebc8bcf07d248ecf628e19f70384a5dc1cb |
Backport failed for Please cherry-pick the changes locally. git fetch origin stable/8.0
git worktree add -d .worktree/backport-9854-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-9854-to-stable/8.0
git checkout -b backport-9854-to-stable/8.0
ancref=$(git merge-base b270a213635b7f5dc2b1497c02acaec1057b149d 0fd1eebc8bcf07d248ecf628e19f70384a5dc1cb)
git cherry-pick -x $ancref..0fd1eebc8bcf07d248ecf628e19f70384a5dc1cb |
Description
This PR fixes a bug where deserialization of exporter configuration was case sensitive, such that a property
NUMBEROFSHARDS
would not map to an instance fieldnumberOfShards
.This isn't perfect, as now if we have two fields with the same name but different cases, only one of them will be set, using the latest value in the map (where the order is undefined). It should solve the most common cases however.
Related issues
closes #7628
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Please refer to our review guidelines.