-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix(elasticsearch sink): Encode bulk action parameters as JSON #21293
base: master
Are you sure you want to change the base?
Conversation
They are currently using string templating which, if there are special characters in the value, will end up creating an invalid JSON payload; an issue that can be difficult to track down. This happened in #21288. Signed-off-by: Jesse Szwedko <[email protected]>
Datadog ReportBranch report: ❌ 9 Failed (0 Known Flaky), 2223 Passed, 0 Skipped, 1m 31.64s Total Time ❌ Failed Tests (9)
|
"TYPE\n", | ||
false, | ||
&DocumentMetadata::Id("ID\n".to_string()), | ||
); |
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.
Should we add an .unwrap()
here?
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.
Mmm, yeah, I just used the pattern the other tests here are using, but yes, it would make sense to use unwrap
on all of these
Note: Tests fail due the order of keys in the objects. The data look identical otherwise. |
…csearch-message-encoding
Signed-off-by: Jesse Szwedko <[email protected]>
They are currently using string templating which, if there are special characters in the value, will
end up creating an invalid JSON payload; an issue that can be difficult to track down. This happened
in #21288.
Signed-off-by: Jesse Szwedko [email protected]