-
Notifications
You must be signed in to change notification settings - Fork 8
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
Use UDF specific names for server info file #35
Conversation
Signed-off-by: Sreekanth <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this now run against numaflow main branch?
let shutdown = async { | ||
shutdown_rx.await.unwrap(); | ||
}; | ||
let task = tokio::spawn(async move { server.start_with_shutdown(shutdown).await }); | ||
|
||
tokio::time::sleep(Duration::from_millis(50)).await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i forget why we need this sleep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was done just to ensure the server is started completely before sending requests to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we write a small utility func to do a ping
and use that? else there could be flaky tests in the future. we do not have to do this in this PR.
I haven't tried running a full pipeline since the only change was the server info filename. I copied it from numaflow-go. |
Tested with
And ran a source that pulls from kafka:
|
This change also makes the
start_with_shutdown
API for all servers a bit more nicer.