chore(topology): split build_pieces into smaller functions#17037
Merged
StephenWakely merged 5 commits intomasterfrom Apr 5, 2023
Merged
chore(topology): split build_pieces into smaller functions#17037StephenWakely merged 5 commits intomasterfrom
build_pieces into smaller functions#17037StephenWakely merged 5 commits intomasterfrom
Conversation
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
✅ Deploy Preview for vector-project canceled.
|
github-actions
bot
added
the
domain: topology
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
Datadog ReportBranch report: ✅ |
Regression Detector ResultsRun ID: c8301ea3-7d49-4e9a-a6f2-a99b97d6e839 ExplanationA regression test is an integrated performance test for The table below, if present, lists those experiments that have experienced a statistically significant change in mean optimization goal performance between baseline and comparison SHAs with 90.00% 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 ±5.00% change in their mean optimization goal are discarded. An experiment is erratic if its coefficient of variation is greater than 0.1. The abbreviated table will be omitted if no interesting change is observed. No interesting changes in experiment optimization goals with confidence ≥ 90.00% and |Δ mean %| ≥ 5.00%. Fine details of change detection per experiment.
|
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
Regression Detector ResultsRun ID: 65e75109-4dd4-40fb-ba08-6e2364d60792 ExplanationA regression test is an integrated performance test for The table below, if present, lists those experiments that have experienced a statistically significant change in mean optimization goal performance between baseline and comparison SHAs with 90.00% 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 ±5.00% change in their mean optimization goal are discarded. An experiment is erratic if its coefficient of variation is greater than 0.1. The abbreviated table will be omitted if no interesting change is observed. No interesting changes in experiment optimization goals with confidence ≥ 90.00% and |Δ mean %| ≥ 5.00%. Fine details of change detection per experiment.
|
spencergilbert
approved these changes
Apr 4, 2023
bruceg
reviewed
Apr 4, 2023
Member
bruceg
left a comment
bruceg
left a comment
There was a problem hiding this comment.
LGTM except for the long argument lists, which wouldn't be a big deal except that most of them are repeated.
src/topology/builder.rs
Outdated
Comment on lines
+159
to
+190
| let source_tasks = build_sources( | ||
| config, | ||
| diff, | ||
| &mut shutdown_coordinator, | ||
| &mut errors, | ||
| &mut outputs, | ||
| &mut tasks, | ||
| ) | ||
| .await; | ||
|
|
||
| build_transforms( | ||
| config, | ||
| diff, | ||
| enrichment_tables, | ||
| &mut errors, | ||
| &mut inputs, | ||
| &mut outputs, | ||
| &mut tasks, | ||
| ) | ||
| .await; | ||
|
|
||
| build_sinks( | ||
| config, | ||
| diff, | ||
| &mut errors, | ||
| buffers, | ||
| &mut inputs, | ||
| &mut healthchecks, | ||
| &mut tasks, | ||
| &mut detach_triggers, | ||
| ) | ||
| .await; |
Member
There was a problem hiding this comment.
I'd like to see these repeated parameters moved into a common struct or something, and then the functions could be member functions of that struct. Notable, all of the following are repeated: config, diff, errors, inputs, outputs, and tasks.
Contributor
Author
There was a problem hiding this comment.
I really like this. It has made the diffs for the PR much harder to read, but it is largely just moving code around..
Member
There was a problem hiding this comment.
It gets a lot smaller when checking off to hide whitespace changes.
src/topology/builder.rs
Outdated
Comment on lines
+205
to
+215
| let pieces = Pieces { | ||
| inputs, | ||
| outputs: finalized_outputs, | ||
| tasks, | ||
| source_tasks, | ||
| healthchecks, | ||
| shutdown_coordinator, | ||
| detach_triggers, | ||
| }; | ||
|
|
||
| Ok(pieces) |
Member
There was a problem hiding this comment.
nit: shorter:
Suggested change
| let pieces = Pieces { | |
| inputs, | |
| outputs: finalized_outputs, | |
| tasks, | |
| source_tasks, | |
| healthchecks, | |
| shutdown_coordinator, | |
| detach_triggers, | |
| }; | |
| Ok(pieces) | |
| Ok(Pieces { | |
| inputs, | |
| outputs: finalized_outputs, | |
| tasks, | |
| source_tasks, | |
| healthchecks, | |
| shutdown_coordinator, | |
| detach_triggers, | |
| }) |
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
✅ Deploy Preview for vrl-playground canceled.
|
Regression Detector ResultsRun ID: 484f607c-3adc-440a-804e-4fbf104cc5d5 ExplanationA regression test is an integrated performance test for The table below, if present, lists those experiments that have experienced a statistically significant change in mean optimization goal performance between baseline and comparison SHAs with 90.00% 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 ±5.00% change in their mean optimization goal are discarded. An experiment is erratic if its coefficient of variation is greater than 0.1. The abbreviated table will be omitted if no interesting change is observed. Changes in experiment optimization goals with confidence ≥ 90.00% and |Δ mean %| ≥ 5.00%:
Fine details of change detection per experiment.
|
bruceg
approved these changes
Apr 5, 2023
StephenWakely
added a commit
that referenced
this pull request
Apr 17, 2023
* Revert "chore(topology): Transform outputs hash table of OutputId -> Definition (#17059)" This reverts commit 1bdb24d. * Revert "chore(topology): split `build_pieces` into smaller functions (#17037)" This reverts commit 6e6f1eb. * Revert "enhancement(topology): Update transforms to handle multiple definitions (#16793)" This reverts commit e19f4fc. Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com> --------- Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
neuronull
pushed a commit
that referenced
this pull request
Apr 17, 2023
* Revert "chore(topology): Transform outputs hash table of OutputId -> Definition (#17059)" This reverts commit 1bdb24d. * Revert "chore(topology): split `build_pieces` into smaller functions (#17037)" This reverts commit 6e6f1eb. * Revert "enhancement(topology): Update transforms to handle multiple definitions (#16793)" This reverts commit e19f4fc. Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com> --------- Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
neuronull
pushed a commit
that referenced
this pull request
Apr 19, 2023
* Revert "chore(topology): Transform outputs hash table of OutputId -> Definition (#17059)" This reverts commit 1bdb24d. * Revert "chore(topology): split `build_pieces` into smaller functions (#17037)" This reverts commit 6e6f1eb. * Revert "enhancement(topology): Update transforms to handle multiple definitions (#16793)" This reverts commit e19f4fc. Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
neuronull
pushed a commit
that referenced
this pull request
Apr 19, 2023
* Revert "chore(topology): Transform outputs hash table of OutputId -> Definition (#17059)" This reverts commit 1bdb24d. * Revert "chore(topology): split `build_pieces` into smaller functions (#17037)" This reverts commit 6e6f1eb. * Revert "enhancement(topology): Update transforms to handle multiple definitions (#16793)" This reverts commit e19f4fc. Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
StephenWakely
added a commit
that referenced
this pull request
Apr 27, 2023
* Revert "Revert "enhancement(topology): Update transforms to handle multiple definitions (#16793)"" This reverts commit 5dc20f3. * Revert "Revert "chore(topology): split `build_pieces` into smaller functions (#17037)"" This reverts commit 0e11bc3. * Revert "Revert "chore(topology): Transform outputs hash table of OutputId -> Definition (#17059)"" This reverts commit 8916ec1. --------- Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A tiny refactor, this function was getting a bit too long.