Skip to content

Conversation

sicoyle
Copy link
Contributor

@sicoyle sicoyle commented May 5, 2023

Thank you for helping make the Dapr documentation better!

Please follow this checklist before submitting:

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Read the contribution guide
  • Commands include options for Linux, MacOS, and Windows within codetabs
  • New file and folder names are globally unique
  • Page references use shortcodes instead of markdown or URL links
  • Images use HTML style and have alternative text
  • Places where multiple code/command options are given have codetabs

In addition, please fill out the following to help reviewers understand this pull request:

Description

Add docs to support the extension of the service invocation API to allow for the invocation of non-Dapr endpoints.

Issue reference

#3365

…ndpoints

Signed-off-by: Samantha Coyle <sam@diagrid.io>
@sicoyle sicoyle requested review from a team as code owners May 5, 2023 00:23
@sicoyle sicoyle changed the base branch from v1.10 to v1.11 May 5, 2023 00:24
@sicoyle
Copy link
Contributor Author

sicoyle commented May 5, 2023

non-Dapr endpoints vs non-Daprized endpoints? Which do we like better for docs?

@hhunter-ms
Copy link
Contributor

non-Dapr endpoints vs non-Daprized endpoints? Which do we like better for docs?

@sicoyle I personally like "non-Dapr endpoints". @msfussell what do you think?

@msfussell
Copy link
Member

msfussell commented May 8, 2023

non-Dapr endpoints vs non-Daprized endpoints? Which do we like better for docs?

@sicoyle I personally like "non-Dapr endpoints". @msfussell what do you think?

Let's use non-Dapr endpoints.

Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

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

Lets start with these changes.

<img src="/images/service-invocation-overview-non-daprized-endpoint.png" width=800 alt="Diagram showing the steps of service invocation to non-Daprized endpoints">

1. Service A makes an HTTP call targeting Service B, a non-Daprized endpoint. The call goes to the local Dapr sidecar.
2. Dapr discovers Service B's location using the [name resolution component]({{< ref supported-name-resolution >}}) which is running on the given [hosting platform]({{< ref "hosting" >}}).
Copy link
Member

Choose a reason for hiding this comment

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

The Naming Component is not used here, that is for appId. You should update this to talk about resolving with either a name HTTPEndpoint resource or FQDM URL. I would also update this to have an picture that is a HTTP resource

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated... however, I mostly just removed the name resolution component reference. My thinking is Service B is simply a non-Dapr service, so it is still fair game to visualize it as a "Service" icon, but the description now denotes this to be discovered either using an HTTPEndpoint or FQDN URL.


### HTTP service invocation to non-Daprized endpoints
If you are in a brownfield scenario or simply have existing non-Daprized endpoints that you want to invoke, then you can do so using the service invocation API.
By defining a Dapr HTTPEndpoint, you declaratively define a way to interact with a non-Daprized endpoint. You then use the existing service invocation URL, while replacing the internal `appid` with the HTTPEndpoint name, and can use the existing service invocation API as is. This allows for you to also use the HTTPEndpoint name as the value if you use the `dapr-app-id` header. Alternatively, you can place a non-Daprized endpoint URL directly into the service invocation URL where you would place the `appid` or HTTPEndpoint name.
Copy link
Member

Choose a reason for hiding this comment

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

Move this into the How To Topic. This is where the details are needed. Also rewrite just to simply say how to use the HTTPEndpoint or FQDM URL and not try to say this is a replacement for appId. Blend this in with the order of precedence detail in the How To Topic.

Suggested change
By defining a Dapr HTTPEndpoint, you declaratively define a way to interact with a non-Daprized endpoint. You then use the existing service invocation URL, while replacing the internal `appid` with the HTTPEndpoint name, and can use the existing service invocation API as is. This allows for you to also use the HTTPEndpoint name as the value if you use the `dapr-app-id` header. Alternatively, you can place a non-Daprized endpoint URL directly into the service invocation URL where you would place the `appid` or HTTPEndpoint name.
By defining a HTTPEndpoint resource, you declaratively define a way to interact with a non-Daprized endpoint.

@msfussell
Copy link
Member

@sicoyle - You need to perform a new pull on Dapr v1.11 branch for the build to succeed.

@github-actions
Copy link

Stale PR, paging all reviewers

@github-actions github-actions bot added the stale label May 15, 2023
msfussell and others added 5 commits May 15, 2023 08:40
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
Signed-off-by: Samantha Coyle <sam@diagrid.io>
@sicoyle
Copy link
Contributor Author

sicoyle commented May 15, 2023

@msfussell could you please remove the stale label?

@yaron2 yaron2 removed the stale label May 15, 2023
@yaron2
Copy link
Member

yaron2 commented May 15, 2023

@msfussell could you please remove the stale label?

Removed.

Editing of the docs

Signed-off-by: Mark Fussell <markfussell@gmail.com>
Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

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

Good updates. Getting closer. I did one larger edit that you should review, so pull down the latest from your updated branch

Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

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

Good updates. Getting closer. I did one larger edit that you should review, so pull down the latest from your updated branch

msfussell

This comment was marked as duplicate.

Signed-off-by: Samantha Coyle <sam@diagrid.io>
Copy link
Contributor

@hhunter-ms hhunter-ms left a comment

Choose a reason for hiding this comment

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

quick grammar review

Signed-off-by: Samantha Coyle <sam@diagrid.io>
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