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.

Sign up for GitHub

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

Jump to bottom

modify Service Invocation sample #902

Merged
merged 3 commits into from
Feb 11, 2024
Merged

modify Service Invocation sample #902

merged 3 commits into from
Feb 11, 2024

Conversation

MregXN
Copy link
Member

@MregXN MregXN commented Sep 7, 2023

Description

Currently the Service Invocation sample in quickstarts are using http request directly. Now they are changed to use dapr client.

Issue reference

Please reference the issue #888 .

@MregXN MregXN force-pushed the use-daprclient branch 2 times, most recently from 0be9710 to 99be3ca Compare September 8, 2023 05:06
@msfussell
Copy link
Member

@MregXN A - Appreciate you doing this PR. However for invoke method we only recommend using the http proxy (bring your own client) because this interoperates better with existing code. The SDK is not helpful in this scenario. The invoke is no longer recommend.

@MregXN
Copy link
Member Author

MregXN commented Sep 25, 2023

@MregXN A - Appreciate you doing this PR. However for invoke method we only recommend using the http proxy (bring your own client) because this interoperates better with existing code. The SDK is not helpful in this scenario. The invoke is no longer recommend.

Sorry for my misunstanding. In reference to your discription in issue #888, which specific SDK functions you wish to utilize?

paulyuk
paulyuk requested changes Sep 27, 2023
View reviewed changes
Copy link
Contributor

@paulyuk paulyuk left a comment

Choose a reason for hiding this comment

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

Please see my inline comments that hopefully explain what @msfussell and I were thinking.

service_invocation/csharp/http/checkout/Program.cs Outdated Show resolved Hide resolved
service_invocation/csharp/http/checkout/Program.cs Outdated Show resolved Hide resolved
@MregXN MregXN marked this pull request as draft November 21, 2023 03:04
@MregXN MregXN closed this Nov 21, 2023
@MregXN MregXN reopened this Nov 21, 2023
@MregXN
add dependency
cb2075f 
Signed-off-by: MregXN <[email protected]>
@MregXN MregXN marked this pull request as ready for review November 21, 2023 06:50
@MregXN
Copy link
Member Author

MregXN commented Nov 21, 2023

@msfussell @paulyuk ready ro review

paulyuk
paulyuk approved these changes Feb 11, 2024
View reviewed changes
Copy link
Contributor

@paulyuk paulyuk left a comment

Choose a reason for hiding this comment

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

LGTM. thank you for the contribution and iterations

@paulyuk paulyuk merged commit 583e9b3 into dapr:master Feb 11, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulyuk paulyuk paulyuk approved these changes

Assignees

@MregXN MregXN

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MregXN @msfussell @paulyuk