-
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
chore(sinks, codecs): Remove legacy EncodingConfiguration
#13518
Conversation
✅ Deploy Preview for vector-project ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
EncodingConfiguration
EncodingConfiguration
Signed-off-by: Pablo Sichert <[email protected]>
Signed-off-by: Pablo Sichert <[email protected]>
Signed-off-by: Pablo Sichert <[email protected]>
d1ccfb5
to
2ab8a6e
Compare
Signed-off-by: Pablo Sichert <[email protected]>
Signed-off-by: Pablo Sichert <[email protected]>
Signed-off-by: Pablo Sichert <[email protected]>
Signed-off-by: Pablo Sichert <[email protected]>
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.
These are some good diff stats 😄
I'll go over this in more detail soon, but one thing that I think we need to add here is a note to the 0.24.0 upgrade guide explaining how to migrate.
website/content/en/highlights/2020-07-10-add-reduce-transform.md
Outdated
Show resolved
Hide resolved
@@ -171,10 +171,8 @@ components: sinks: [Name=string]: { | |||
encoding: { | |||
description: """ | |||
Configures the encoding specific sink behavior. | |||
|
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.
🎉
The deprecation and migration path has already been covered in the I think we can add a short note in the |
Ah, true, that would work. We can announce the breaking change and link to the deprecation warning from 0.23.0 and mention it includes the migration details. |
Soak Test ResultsBaseline: 176060c ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
…text Signed-off-by: Pablo Sichert <[email protected]>
Signed-off-by: Pablo Sichert <[email protected]>
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.
This generally looks good to me.
I left some non-blocking nits/questions/thoughts, and found one spot you missed when converting examples to use encoding.codec
but other than that... 👍🏻
Signed-off-by: Pablo Sichert <[email protected]>
Signed-off-by: Pablo Sichert <[email protected]>
Soak Test ResultsBaseline: 176060c ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
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.
I mostly scanned through, but nothing jumped out. Just a couple of notes on the upgrade guide, otherwise I think this is good!
@@ -0,0 +1,360 @@ | |||
#![deny(missing_docs)] |
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.
In the future, it would be useful to call out which code was just moved to try to focus reviews. This could be done in the form of PR comments or separating out refactoring commits.
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.
True - I wasn't very considerate about that! Would save quite some time.
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.
No worries! I checked first to see if this was new code and found it had just been moved.
website/content/en/highlights/2022-08-16-0-24-0-upgrade-guide.md
Outdated
Show resolved
Hide resolved
website/content/en/highlights/2022-08-16-0-24-0-upgrade-guide.md
Outdated
Show resolved
Hide resolved
website/content/en/highlights/2022-08-16-0-24-0-upgrade-guide.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Jesse Szwedko <[email protected]>
…-legacy-encoding Signed-off-by: Pablo Sichert <[email protected]>
website/content/en/highlights/2022-08-16-0-24-0-upgrade-guide.md
Outdated
Show resolved
Hide resolved
Signed-off-by: Pablo Sichert <[email protected]>
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.
🎉
Soak Test ResultsBaseline: 7cc3a80 ExplanationA soak test is an integrated performance test for vector in a repeatable rig, with varying configuration for vector. What follows is a statistical summary of a brief vector run for each configuration across SHAs given above. The goal of these tests are to determine, quickly, if vector performance is changed and to what degree by a pull request. Where appropriate units are scaled per-core. The table below, if present, lists those experiments that have experienced a statistically significant change in their throughput performance between baseline and comparision SHAs, with 90.0% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±8.87% change in mean throughput are discarded. An experiment is erratic if its coefficient of variation is greater than 0.3. The abbreviated table will be omitted if no interesting changes are observed. No interesting changes in throughput with confidence ≥ 90.00% and absolute Δ mean >= ±8.87%: Fine details of change detection per experiment.
|
Related to #9459.
Closes #12162.
Closes #12127.
Closes #12134.
Tip: Use the "hide whitespace" option in the files tab when reviewing.