-
Notifications
You must be signed in to change notification settings - Fork 647
Allow DAPR apps without a http endpoint to function correctly #6362
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
Conversation
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.
Looks ok to me.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@davidfowl -- will this make next release? It's a blocker for us. Thx! |
It could, there's still time, my big concern is that we have no automated test coverage for dapr at the moment. |
Spoke to the team and since we have little dapr test coverage at the moment I will merge this into main so that we can do more testing. Worst case is that we can back port to 9.0.1 if it doesn't make this release. |
I did add extend the playground app for it to verify, but yeah, I wish I could help more. Ultimately though, without this fix, the dapr support is very limited (without very disruptive workarounds) |
Can you help verify 9.1.0 builds? |
Yes @davidfowl -- what do you need me to do? |
Verify that there are no regressions in using it with dapr. |
Description
The DAPR hosting
WithDaprSideCar()
expects the application to always expose a http endpoint. In the DAPR world, this is not the expectation. Only apps that expect to work with input bindings or subscribe to a pubsub topic (i.e. receive messages or events) are expected to have an http endpoint.It seems a regression was introduced where an endpoint validation was previously a null check, and now it requires an
IsAllocated
boolean test. I also simplified the tests into patterns and unified the three checks across all threeif
statements checking appPort, appChannelAddress and appProtocol respectively.I added a third "service C" to the playground app to validate that a service without an endpoint is successfully launched.
Unit tests (to come?) -- but they look pretty arcane. Might need some pointers.
Fixes #3687
Checklist
<remarks />
and<code />
elements on your triple slash comments?