Skip to content

Conversation

@yhl25
Copy link
Contributor

@yhl25 yhl25 commented Sep 22, 2025

Closes #203

Signed-off-by: Yashash H L <yashashhl25@gmail.com>
@codecov
Copy link

codecov bot commented Sep 22, 2025

Codecov Report

❌ Patch coverage is 81.48148% with 10 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@4adc44b). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...ain/java/io/numaproj/numaflow/sourcer/Service.java 70.00% 8 Missing and 1 partial ⚠️
...va/io/numaproj/numaflow/shared/ExceptionUtils.java 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #204   +/-   ##
=======================================
  Coverage        ?   59.14%           
  Complexity      ?      482           
=======================================
  Files           ?      152           
  Lines           ?     3388           
  Branches        ?      228           
=======================================
  Hits            ?     2004           
  Misses          ?     1219           
  Partials        ?      165           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vigith vigith marked this pull request as ready for review October 3, 2025 04:04
yhl25 added 3 commits October 3, 2025 17:47
Signed-off-by: Yashash H L <yashashhl25@gmail.com>
Signed-off-by: Yashash H L <yashashhl25@gmail.com>
Comment on lines 65 to 74
// send the message to the observer
observer.send(message);
observer.send(constructMessage(index));
// keep track of the messages read and not acknowledged
messages.put(readIndex, true);
readIndex += 1;
yetToBeAcked.put(index, true);
readIndex.incrementAndGet();
Copy link
Member

Choose a reason for hiding this comment

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

Can we move Integer index = readIndex.incrementAndGet(); from L64 to L70 and remove L74?

Copy link
Member

Choose a reason for hiding this comment

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

Should we move instead of remove Integer index = readIndex.incrementAndGet();? we still want to update the readIndex every time we process a msg right? @yhl25

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we should move, fixed.

Signed-off-by: Yashash H L <yashashhl25@gmail.com>
@yhl25 yhl25 requested a review from KeranYang October 3, 2025 15:11
Signed-off-by: Yashash H L <yashashhl25@gmail.com>
@yhl25 yhl25 merged commit 38734c0 into main Oct 3, 2025
5 checks passed
@yhl25 yhl25 deleted the nack branch October 3, 2025 17:24
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.

Nack Support For User Defined Source

4 participants