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.

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

Adds docs for client initialisation with env variables #992

Merged
merged 4 commits into from
Jan 24, 2024

Conversation

elena-kolevska
Copy link
Contributor

Description

Added a section in the docs on initialising the client and the supported environment variables

Issue reference

dapr/docs#3921

Checklist

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

  • Extended the documentation

N/A

  • Code compiles correctly
  • Created/updated tests

@elena-kolevska elena-kolevska requested review from a team as code owners January 18, 2024 16:53
@elena-kolevska
Copy link
Contributor Author

@cicoyle could you please review this, I'm not very familiar with the java-sdk. Thanks 🙏

without any arguments:

```bash
export DAPR_GRPC_ENDPOINT="mydomain:50051?tls=true"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a small nit: Initialising -> Initializing && standardising -> standardizing. I'm not 100% confident I see the parsing for the tls=true in our codebase. I might've just missed it though. Once this is confirmed, then I think the PR is good to go. Thank you for documenting this 🎉

Copy link
Contributor

Choose a reason for hiding this comment

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

we may be missing a conditional, like:

      if (uri.getQuery() != null && uri.getQuery().contains("tls=true")) {
        insecure = false;
      }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Cassie! I realised I had some copy/paste leftovers from the python-sdk. I fixed that now.
The ?tls query parameter was added in this proposal.
I removed the example for now, so we're not showing the grpc endpoint with "https", and we can come back to this and add it, once the support for tls is added.

Signed-off-by: Elena Kolevska <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
cicoyle
cicoyle previously approved these changes Jan 18, 2024
Copy link
Contributor

@cicoyle cicoyle left a comment

Choose a reason for hiding this comment

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

Thanks for updating the docs 🎉

Copy link

codecov bot commented Jan 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7eb24ff) 77.31% compared to head (edb7752) 77.31%.

❗ Current head edb7752 differs from pull request most recent head 50968fd. Consider uploading reports for the commit 50968fd to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #992   +/-   ##
=========================================
  Coverage     77.31%   77.31%           
  Complexity     1537     1537           
=========================================
  Files           143      143           
  Lines          4642     4642           
  Branches        541      541           
=========================================
  Hits           3589     3589           
  Misses          778      778           
  Partials        275      275           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@artursouza artursouza merged commit a9a09ba into dapr:master Jan 24, 2024
6 of 8 checks passed
@artursouza artursouza added this to the v1.11 milestone Jan 24, 2024
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