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.

Sign up for GitHub

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

Jump to bottom

feat: Bidirectional Streaming for User Defined Source #2056

Merged
merged 49 commits into from
Sep 23, 2024
Merged

feat: Bidirectional Streaming for User Defined Source #2056

merged 49 commits into from
Sep 23, 2024

Conversation

yhl25
Copy link
Contributor

@yhl25 yhl25 commented Sep 12, 2024

No description provided.

yhl25 and others added 24 commits September 10, 2024 12:08
@yhl25
bidi source
f486d49 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
Merge branch 'main' of github.com:numaproj/numaflow into bidi-source
571bc05
@yhl25
fix tests
5ce20aa 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
fix tests
3eece70 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
fix changes
9a10b7a 
Signed-off-by: Yashash H L <[email protected]>
@vigith
chore: minor refactor
ba4f413 
Signed-off-by: Vigith Maurice <[email protected]>
@vigith
feat: introduce startup for initialization and validation
ab4fc8e 
Signed-off-by: Vigith Maurice <[email protected]>
@vigith
chore: sync before i create a new branch
a9cdbc7 
Signed-off-by: Vigith Maurice <[email protected]>
@vigith
chore: code rearrange
a294b6b 
Signed-off-by: Vigith Maurice <[email protected]>
@vigith
chore: fmt
8ce9f5e 
Signed-off-by: Vigith Maurice <[email protected]>
@vigith
chore: reduce visibility
81edc44 
Signed-off-by: Vigith Maurice <[email protected]>
@yhl25
fix tests
5baeef7 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
fix tests
948de1c 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
lint and codegen
2a27add 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
Merge branch 'main' of github.com:numaproj/numaflow into bidi-source
52f5ebe
@yhl25
stop retrying while shutting down
e0ae8ca 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
update ubuntu
10a6508 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
lint
7e86c5f 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
use ubuntu latest
c272caa 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
lint
2fb827f 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
use ubuntu latest
02fca5d 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
update ubuntu version in e2e
56b589c 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
resolve conflicts
8306a77 
Signed-off-by: Yashash H L <[email protected]>
@yhl25
install protoc
15ad19a 
Signed-off-by: Yashash H L <[email protected]>
@codecov Codecov
Copy link

codecov bot commented Sep 15, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 81.99195% with 179 lines in your changes missing coverage. Please review.

Project coverage is 64.53%. Comparing base (669dc18) to head (1d38904).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/sdkclient/source/client.go 18.29% 63 Missing and 4 partials ⚠️
rust/monovertex/src/startup.rs 83.14% 45 Missing ⚠️
rust/monovertex/src/lib.rs 77.45% 23 Missing ⚠️
rust/monovertex/src/metrics.rs 90.14% 14 Missing ⚠️
rust/monovertex/src/forwarder.rs 94.08% 10 Missing ⚠️
rust/monovertex/src/source.rs 96.21% 5 Missing ⚠️
rust/monovertex/src/server_info.rs 73.33% 4 Missing ⚠️
pkg/sdkclient/grpc/grpc_utils.go 0.00% 3 Missing ⚠️
rust/src/bin/main.rs 0.00% 3 Missing ⚠️
rust/monovertex/src/message.rs 87.50% 2 Missing ⚠️
... and 3 more
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2056      +/-   ##
==========================================
+ Coverage   63.87%   64.53%   +0.65%     
==========================================
  Files         321      322       +1     
  Lines       29572    30145     +573     
==========================================
+ Hits        18890    19454     +564     
- Misses       9659     9671      +12     
+ Partials     1023     1020       -3

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

@yhl25 yhl25 force-pushed the bidi-source branch 2 times, most recently from a1f44bf to c4cc509 Compare September 15, 2024 02:37
@yhl25
install latest protoc
529a7e9 
Signed-off-by: Yashash H L <[email protected]>
@yhl25 yhl25 force-pushed the bidi-source branch 2 times, most recently from 033f845 to 899453b Compare September 15, 2024 12:12
@vigith vigith changed the base branch from rpc-streaming to main September 18, 2024 15:23
@vigith vigith marked this pull request as ready for review September 20, 2024 15:10
@vigith vigith mentioned this pull request Sep 20, 2024
Closed
@yhl25
Copy link
Contributor Author

yhl25 commented Sep 20, 2024

I have updated the udsource e2e tests to use docker images from my personal directory, so that the tests don't fail. Once the main repo images are updated to use streaming source I will revert those changes.

KeranYang
KeranYang reviewed Sep 20, 2024
View reviewed changes
pkg/sources/udsource/grpc_udsource.go Outdated
Comment on lines 188 to 190
if err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

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

nit: can directly return err.

test/udsource-e2e/testdata/simple-source-rust.yaml Outdated
Comment on lines 17 to 18
# set it as two pods to be different from the sink such that we can use this pipeline
# to test processing rate is consistent across vertices when they have different replica counts.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# set it as two pods to be different from the sink such that we can use this pipeline
# to test processing rate is consistent across vertices when they have different replica counts.

vigith
vigith reviewed Sep 20, 2024
View reviewed changes
rust/monovertex/Cargo.toml

[dev-dependencies]
tempfile = "3.11.0"
numaflow = { git = "https://github.com/numaproj/numaflow-rs.git", branch = "main" }
numaflow = { git = "https://github.com/numaproj/numaflow-rs.git", branch = "handshake" }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
numaflow = { git = "https://github.com/numaproj/numaflow-rs.git", branch = "handshake" }
numaflow = { git = "https://github.com/numaproj/numaflow-rs.git", branch = "main" }

eventually we should point to a stable release in crates.io

whynowy
whynowy reviewed Sep 21, 2024
View reviewed changes
test/e2e/functional_test.go Outdated
@@ -41,7 +41,7 @@ func (s *FunctionalSuite) TestCreateSimplePipeline() {
w := s.Given().Pipeline("@testdata/simple-pipeline.yaml").
When().
CreatePipelineAndWait()
defer w.DeletePipelineAndWait()
//defer w.DeletePipelineAndWait()
Copy link
Member

Choose a reason for hiding this comment

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

Uncomment it?

yhl25 and others added 2 commits September 21, 2024 14:55
@yhl25
address review comments
4e92bc6 
Signed-off-by: Yashash H L <[email protected]>
@vigith
chore: fix comment
1d38904 
Signed-off-by: Vigith Maurice <[email protected]>
@vigith vigith enabled auto-merge (squash) September 23, 2024 02:42
@vigith vigith merged commit 40e960a into main Sep 23, 2024
25 checks passed
@vigith vigith deleted the bidi-source branch September 23, 2024 02:55
@KeranYang KeranYang mentioned this pull request Oct 2, 2024
Closed
SaniyaKalamkar pushed a commit to SaniyaKalamkar/numaflow that referenced this pull request Jan 19, 2025
@yhl25 @vigith @SaniyaKalamkar
feat: Bidirectional Streaming for User Defined Source (numaproj#2056)
3cf2503 
Signed-off-by: Yashash H L <[email protected]>
Signed-off-by: Vigith Maurice <[email protected]>
Co-authored-by: Vigith Maurice <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KeranYang KeranYang KeranYang left review comments

@whynowy whynowy whynowy left review comments

@vigith vigith vigith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yhl25 @vigith @whynowy @KeranYang