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

Allow publishing cloud events directly #868

Merged
merged 3 commits into from
May 20, 2022

Conversation

suraciii
Copy link
Contributor

@suraciii suraciii commented Apr 24, 2022

Description

Please explain the changes you've made

Allow users to publish cloud events directly, so that users can specify attributes (types, subjects, etc), which are useful for events routing.

This PR added two types, CloudEvent and CloudEvent<TData>.

Usage:

var objectDeletedCloundEvent = new CloudEvent<ObjectDeleted>(new(objectName))
{
	Source = new Uri("https://example.com/storage/" + bucketName),
	Type = "com.example.object.deleted.v2",
	Subject = "mynewfile.jpg",
};
await daprClient.PublishEventAsync("mypubsub", "mytopic", objectDeletedCloundEvent);

The result message content in kafka:

{
	"id": "a4c2d9ee0-275c-4c7b-8de7-0d2e71ff942f",
	"data": {
		"objectName": "myfile.jpg"
	},
	"datacontenttype": "application/json",
	"source": "https://example.com/storage/mybucket",
	"type": "com.example.object.deleted.v2",
	"subject": "mynewfile.jpg",
	"traceid": "00-c01e688aeea66df6a18b5f3f7748c74a-27785df64ee3b34c-01",
	"traceparent": "00-c01e688aeea66df6a18b5f3f7748c74a-27785df64ee3b34c-01",
	"tracestate": "",
	"topic": "mytopic",
	"pubsubname": "mypubsub",
	"specversion": "1.0"
}

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #[issue number]
#773

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: Sun Zhongfeng <suraciii@outlook.com>
@suraciii suraciii marked this pull request as ready for review April 25, 2022 07:16
@suraciii suraciii requested review from a team as code owners April 25, 2022 07:16
Copy link
Contributor

@halspang halspang left a comment

Choose a reason for hiding this comment

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

Thanks for opening this PR! Overall, everything looks good. One thing I think it'd be good to investigate is to see if we can mimic the tests here:

https://github.com/dapr/dotnet-sdk/blob/master/test/Dapr.AspNetCore.IntegrationTest/CloudEventsIntegrationTest.cs

Using the updated client. Can you look into that and see if there's value there?

@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #868 (d20159b) into master (2651832) will increase coverage by 0.03%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##           master     #868      +/-   ##
==========================================
+ Coverage   69.13%   69.16%   +0.03%     
==========================================
  Files         142      143       +1     
  Lines        4649     4657       +8     
  Branches      517      520       +3     
==========================================
+ Hits         3214     3221       +7     
  Misses       1318     1318              
- Partials      117      118       +1     
Flag Coverage Δ
net5 69.12% <92.30%> (+0.03%) ⬆️
net6 69.12% <92.30%> (+0.03%) ⬆️
netcoreapp3.1 69.12% <92.30%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/Dapr.Client/DaprClientGrpc.cs 87.70% <80.00%> (-0.18%) ⬇️
src/Dapr.Client/CloudEvent.cs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2651832...d20159b. Read the comment docs.

@suraciii
Copy link
Contributor Author

suraciii commented May 5, 2022

@halspang Thanks for your review.

I think events published by dapr client are encapsulated by dapr runtime, so tests on test/Dapr.AspNetCore.IntegrationTest/CloudEventsIntegrationTest.cs (the consumer side) should not be able to cover the behavior of dapr client.

Please correct me if I have misunderstood that.

@saber-wang
Copy link
Contributor

Can this RP be merged? This function is required

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.

4 participants