Skip to content

feat: Add Map batch#132

Merged
vigith merged 17 commits intonumaproj:mainfrom
RohanAshar:mapBatch
Aug 1, 2024
Merged

feat: Add Map batch#132
vigith merged 17 commits intonumaproj:mainfrom
RohanAshar:mapBatch

Conversation

@RohanAshar
Copy link
Copy Markdown
Contributor

@RohanAshar RohanAshar commented Jul 25, 2024

rashar and others added 9 commits July 19, 2024 09:37
Signed-off-by: rashar <rohan_ashar@intuit.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
…into mapBatch

Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 25, 2024

Codecov Report

Attention: Patch coverage is 73.91304% with 48 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@3c2ec6f). Learn more about missing BASE report.

Files Patch % Lines
...java/io/numaproj/numaflow/batchmapper/Service.java 74.35% 15 Missing and 5 partials ⚠️
.../java/io/numaproj/numaflow/batchmapper/Server.java 57.77% 16 Missing and 3 partials ⚠️
...java/io/numaproj/numaflow/batchmapper/Message.java 70.00% 3 Missing ⚠️
...maproj/numaflow/batchmapper/DatumIteratorImpl.java 87.50% 1 Missing and 1 partial ⚠️
...a/io/numaproj/numaflow/shared/GrpcServerUtils.java 50.00% 1 Missing and 1 partial ⚠️
...va/io/numaproj/numaflow/batchmapper/Constants.java 0.00% 1 Missing ⚠️
...a/io/numaproj/numaflow/batchmapper/GRPCConfig.java 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #132   +/-   ##
=======================================
  Coverage        ?   59.43%           
  Complexity      ?      361           
=======================================
  Files           ?      120           
  Lines           ?     2384           
  Branches        ?      169           
=======================================
  Hits            ?     1417           
  Misses          ?      837           
  Partials        ?      130           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Member

@KeranYang KeranYang left a comment

Choose a reason for hiding this comment

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

Partial Review. Thanks @RohanAshar for quickly pick this up and create the PR. 👍

Copy link
Copy Markdown
Contributor

@yhl25 yhl25 left a comment

Choose a reason for hiding this comment

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

As discussed, let's use threads instead of Akka actors. Since we don't see much benefit in using actors over threads and also to be consistent with the current sink implementation.

Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
@RohanAshar RohanAshar requested review from KeranYang and yhl25 July 30, 2024 16:14
Copy link
Copy Markdown
Member

@KeranYang KeranYang left a comment

Choose a reason for hiding this comment

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

@RohanAshar one more comment and this is probably an unknown unknown to you - we have a build-push workflow, which automatically builds the example image using the latest change and push it to quay.io. Most of our examples are built into an image and used by numaflow e2e tests, having the auto image update help us ensure the SDK changes are validated by numaflow e2e. As we are going to use the example in the PR for batch map java e2e test, we should add it to the workflow yaml file. https://github.com/numaproj/numaflow-java/blob/main/development.md#adding-a-new-example

Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Copy link
Copy Markdown
Member

@KeranYang KeranYang left a comment

Choose a reason for hiding this comment

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

LGTM. I am approving with one minor comment. Please monitor the build-push workflow and enable e2e test on numaflow repo. Thanks!

@KeranYang
Copy link
Copy Markdown
Member

@RohanAshar I have manually created the repository https://quay.io/repository/numaio/numaflow-java/batch-map-example and granted the CI account permission to push the image. Should be good to monitor the build-push workflow after merging.

@vigith vigith merged commit 7997724 into numaproj:main Aug 1, 2024
KeranYang added a commit to KeranYang/numaflow-java that referenced this pull request Jan 22, 2025
Signed-off-by: rashar <rohan_ashar@intuit.com>
Signed-off-by: RohanAshar <rohanashar1@gmail.com>
Co-authored-by: rashar <rohan_ashar@intuit.com>
Co-authored-by: Keran Yang <yangkr920208@gmail.com>
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.

Java-SDK single-server info for map Java SDK for BatchMap

4 participants