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

feat(client): Add option to make receive callback blocking #771

Conversation

smilence-yu
Copy link
Contributor

@smilence-yu smilence-yu commented May 8, 2022

Addresses #770

  • Add an option for the callback on receiver to be blocking or nonblocking
  • Updated blocking_test to ensure when pollRoutines is 1 AND it's blocking, the events are processed in serialized manner.

Manual Testing

Run the example receiver code: https://gist.github.com/liu-cong/a132fe8629715419c7a58ee20001d071
It retrieves the Ce-Sleep attribute from the event and sleep for that period of time.

Then send a few events to the receiver using curl,

curl -v localhost:8080
  -X POST \
  -H "Ce-Sleep: 20s" \
  -H "Ce-Id: 1" \
  -H "Ce-Specversion: 1.0" \
  -H "Ce-Type: type" \
  -H "Ce-Source: source" \
  -H "Content-Type: application/json" \
  -d '{"msg":"Test!"}

Without blockingCallback flag

cloudevents.NewClient(p, client.WithPollGoroutines(1))

Sent 3 events, finish at the same time

image

With blockingCallback flag

cloudevents.NewClient(p, client.WithBlockingCallback(), client.WithPollGoroutines(1))

image

Finished one after another

Signed-off-by: James Yu jyu@confluent.io

Signed-off-by: James Yu <jyu@confluent.io>
Signed-off-by: James Yu <jyu@confluent.io>
@smilence-yu smilence-yu force-pushed the 770-add-option-to-make-callback-blocking branch from bc19cf1 to 81a6caa Compare May 8, 2022 03:52
@n3wscott
Copy link
Member

n3wscott commented May 9, 2022

The lint error seems to be related to go 1.18 and the linter. Not related to this PR.

Copy link
Member

@n3wscott n3wscott left a comment

Choose a reason for hiding this comment

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

Nice work, simple fix, thanks a bunch!!

LGTM

@n3wscott n3wscott closed this May 9, 2022
@n3wscott n3wscott reopened this May 9, 2022
@n3wscott n3wscott merged commit 66eacad into cloudevents:main May 9, 2022
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.

None yet

2 participants