8000 Allow DAPR apps without a http endpoint to function correctly by oising · Pull Request #6362 · dotnet/aspire · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 8 commits into from
Oct 25, 2024

Conversation

oising
Copy link
Contributor
@oising oising commented Oct 17, 2024

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 three if 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

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • 8000
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
      • Link to aspire-docs issue:
    • No

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 17, 2024
@oising oising marked this pull request as ready for review October 17, 2024 21:34
Copy link
Contributor
@philliphoff philliphoff left a 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.

@davidfowl
Copy link
Member

/azp run

Copy link
Azure Pipelines successfully started running 1 pipeline(s).

@davidfowl
Copy link
Member

/azp run

Copy link
Azure Pipelines successfully started running 1 pipeline(s).

@oising
Copy link
Contributor Author
oising commented Oct 24, 2024

@davidfowl -- will this make next release? It's a blocker for us. Thx!

@davidfowl
Copy link
Member

It could, there's still time, my big concern is that we have no automated test coverage for dapr at the moment.

@davidfowl
Copy link
Member

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.

@davidfowl davidfowl merged commit 2f5d5de into dotnet:main Oct 25, 2024
9 checks passed
@oising
Copy link
Contributor Author
oising commented Oct 25, 2024

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)

@davidfowl
Copy link
Member

Can you help verify 9.1.0 builds?

@oising
Copy link
Contributor Author
oising commented Oct 28, 2024

Can you help verify 9.1.0 builds?

Yes @davidfowl -- what do you need me to do?

@oising oising deleted the dapr-hosting-optional-endpoint branch October 28, 2024 13:16
@davidfowl
Copy link
Member

Verify that there are no regressions in using it with dapr.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2024
@github-actions github-actions bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Mar 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How can I add project without endpoints with Dapr sidecar?
3 participants
0