Skip to content
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

Remove Comparator and Jupyter since it will be replaced by OTEL and Analytics Engine #409

Merged
merged 5 commits into from
Nov 12, 2023

Conversation

sumobrian
Copy link
Collaborator

Description

Removing references to Comparator and Jupyter in the project. Future builds should exclude these containers and stacks. This pull request can serve as a resource for future projects involving Comparator. We will apply our insights to improve comparison processes using OpenTelemetry and an analytics engine. This approach aims to generate visualizations and tools for evaluating updates, migrations, or comparisons more effectively.

Testing

Verified no unit tests were broken.
Verified Docker Solution end-to-end test using OpenSearch benchmarks ran as expected.

Check List

  • New functionality includes testing
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Merging #409 (22822d8) into main (8710f72) will increase coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head 22822d8 differs from pull request most recent head 0de0a3e. Consider uploading reports for the commit 0de0a3e to get more accurate results

@@             Coverage Diff              @@
##               main     #409      +/-   ##
============================================
+ Coverage     60.41%   60.43%   +0.01%     
+ Complexity      673      670       -3     
============================================
  Files            78       78              
  Lines          3241     3235       -6     
  Branches        300      299       -1     
============================================
- Hits           1958     1955       -3     
+ Misses         1087     1084       -3     
  Partials        196      196              
Flag Coverage Δ
unittests 60.43% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...atahandlers/http/HttpJsonTransformingConsumer.java 71.26% <ø> (ø)
...ch/migrations/replay/SourceTargetCaptureTuple.java 0.00% <0.00%> (ø)
.../opensearch/migrations/replay/TrafficReplayer.java 15.55% <0.00%> (-0.38%) ⬇️

... and 1 file with indirect coverage changes

@@ -101,7 +47,7 @@ javaContainerServices.each { projectName, dockerImageName ->
CommonUtils.createDockerfile(project, projectName, baseImageProjectOverrides, dockerFilesForExternalServices)
}

(javaContainerServices + trafficComparatorServices).forEach { projectName, dockerImageName ->
(javaContainerServices).forEach { projectName, dockerImageName ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very minor, but the parens here are now unnecessary

Copy link
Collaborator

@gregschohn gregschohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the change on stdout, can you please cut a branch with the trafficComparator & Jupyter. We can merge mainline to it if/when we like. That let's us remember where it is, keep it relatively warm, yet not have it cluttering the mainline

@@ -58,7 +58,7 @@ public class HttpJsonTransformingConsumer<R> implements IPacketFinalizingConsume
private final List<List<Integer>> chunkSizes;
// This is here for recovery, in case anything goes wrong with a transformation & we want to
// just dump it directly. Notice that we're already storing all of the bytes until the response
// comes back so that we can format the output that goes to the comparator. These should be
// comes back so that we can format the output. These should be
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reformat to not make it look like you deleted something :)

@sumobrian sumobrian merged commit b011fd2 into main Nov 12, 2023
8 of 9 checks passed
@sumobrian sumobrian deleted the remove_jupyter_and_comparator branch November 16, 2023 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants