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

Propagate content type for pubsub data. #2493

Merged
merged 2 commits into from
Nov 26, 2020

Conversation

artursouza
Copy link
Member

Description

Propagate content type for pubsub data.

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: #2080

Checklist

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

@artursouza
Copy link
Member Author

/ok-to-test

@dapr-bot
Copy link
Collaborator

Found the available test cluster - dapr-aks-e2e-05 for linux. Please check the build status.

@dapr-bot
Copy link
Collaborator

Found the available test cluster - dapr-aks-e2e-06 for windows. Please check the build status.

wcs1only
wcs1only previously approved these changes Nov 26, 2020
@wcs1only wcs1only dismissed their stale review November 26, 2020 01:40

Lint/unit tests build failed

@dapr-bot
Copy link
Collaborator

Congrats! All end-to-end tests have passed on linux. Thanks for your contribution!

@codecov
Copy link

codecov bot commented Nov 26, 2020

Codecov Report

Merging #2493 (ec5965f) into master (bf73765) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2493      +/-   ##
==========================================
- Coverage   57.10%   57.06%   -0.05%     
==========================================
  Files          80       80              
  Lines        7041     7043       +2     
==========================================
- Hits         4021     4019       -2     
- Misses       2743     2747       +4     
  Partials      277      277              
Impacted Files Coverage Δ
pkg/grpc/api.go 58.02% <100.00%> (+0.09%) ⬆️
pkg/http/api.go 90.61% <100.00%> (+0.01%) ⬆️
pkg/placement/membership.go 51.06% <0.00%> (-2.84%) ⬇️

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 bf73765...ec5965f. Read the comment docs.

@artursouza
Copy link
Member Author

/ok-to-test

@dapr-bot
Copy link
Collaborator

Found the available test cluster - dapr-aks-e2e-07 for linux. Please check the build status.

@dapr-bot
Copy link
Collaborator

Found the available test cluster - dapr-aks-e2e-08 for windows. Please check the build status.

@dapr-bot
Copy link
Collaborator

End-to-end tests failed on windows. Please check the build logs

@dapr-bot
Copy link
Collaborator

Found the available test cluster - dapr-aks-e2e-06 for linux. Please check the build status.

@dapr-bot
Copy link
Collaborator

Congrats! All end-to-end tests have passed on linux. Thanks for your contribution!

@dapr-bot
Copy link
Collaborator

End-to-end tests cancelled on linux. Please check the build logs

@dapr-bot
Copy link
Collaborator

Congrats! All end-to-end tests have passed on windows. Thanks for your contribution!

@artursouza artursouza merged commit f92b2a0 into dapr:master Nov 26, 2020
@@ -178,6 +178,9 @@ message PublishEventRequest {

// The data which will be published to topic.
bytes data = 3;

// The content type for the data (optional).
string dataContentType = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow the existing convention. Member naming is snake case, that is what protobuf official guideline recommends. Then Protoc will generate the member based on language it could be camel case or snake case depending on the language.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will need to fix this. Thanks.

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.

Support arbitrary datacontenttype in publish/subscribe
4 participants