search

dapr / quickstarts

Dapr quickstart code samples and tutorials showcasing core Dapr capabilities
Apache License 2.0
1.02k stars 516 forks source link

Updating `hello-kubernetes` quickstart to support Endpoint env vars #1030

Closed salaboy closed 1 week ago

salaboy commented 1 month ago

Description

I've updated the examples to use the DAPR_HTTP_ENDPOINT and DAPR_GRPC_ENDPOINT environment variables that are used by the SDKs. If DAPR_HTTP_PORT needs to be also supported I will need some help to validate the code.

I am not sure how images are created for these applications, help is appreciated to validate these changes.

Issue reference

We strive to have all PRs 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: #1029

Checklist

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

salaboy commented 1 month ago

Team, I am not entirely sure how to run the javascript validation locally.. any hints?

salaboy commented 3 weeks ago

@paulyuk can you help me out here? I wonder if you can advise me on these changes. I can see the Javascript one failing, but it is not clear to me why.

paulyuk commented 3 weeks ago

@salaboy definitely do not worry about Javascript* which has nothing to do with this change. I am separately investigating that. I just kicked off some fresh workflow/action runs in master to see what is going on. Tutorials is the most fragile part of quickstarts - complex and sometimes legacy.

paulyuk commented 3 weeks ago

I see what looks like a timing issue in Hello-Kubernetes where the nodeapp is not started for about 34 loops (seconds) and then starts responding. It looks like some timing issue before the deployment is ready. I have not figured out why yet, just sharing what I'm seeing.

Here is exactly where it fails https://github.com/dapr/quickstarts/actions/runs/9571684813/job/26389335806

paulyuk commented 3 weeks ago

Now I'm running on my local machine simply using Kind + dapr init -k --dev, and then dapr run -f . -k per readme. It fails in the same way, with ~31 tries failing to find and invoke nodeapp and then it starts working after that time. Those failures will fail the test. We need to focus on the timing and why the first 31 invokes fail.

paulyuk commented 3 weeks ago

I just went all the way back to Dapr 1.11 repo and runtime. The issue repro happens all the way back til then. I believe this is a simple timing issue exposed by dapr.yaml (multi app run). because both services (nodeapp server and pythonapp client) start super fast, and expect to communicate. But in reality nodeapp is not ready for ~31 seconds, and without a readiness check it will fail fast over and over, and the test will break.

I'd love to have a readiness check that can be leveraged by dapr.yaml and the app. If not we'll have to think of workarounds, which would refactor dapr.yaml or revert back to individual dapr run commands that build in a human wait for readiness.

msfussell commented 2 weeks ago

@salaboy - The code changes look good. Are you also going to update the text in the readme to have a section on using Dapr Shared with these applications?

salaboy commented 2 weeks ago

That text will be in the Dapr Shared repo that has that tutorial. We shouldn’t complicate any further the Kubernetes example here.

On Tue, 25 Jun 2024 at 21:40, Mark Fussell @.***> wrote:

@salaboy https://github.com/salaboy - The code changes look good. Are you also going to update the text in the readme to have a section on using Dapr Shared with these applications?

— Reply to this email directly, view it on GitHub https://github.com/dapr/quickstarts/pull/1030#issuecomment-2189927500, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACCMXWW4MW5JVJXRWXZQC3ZJHISBAVCNFSM6AAAAABI4XOBPKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBZHEZDONJQGA . You are receiving this because you were mentioned.Message ID: @.***>