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

Add APP_PROTOCOL env var for app #1318

Merged
merged 3 commits into from
Aug 7, 2023

Conversation

ItalyPaleAle
Copy link
Contributor

This PR is the companion of dapr/dapr#6512 which makes the same change in the Dapr Injector.

It adds the APP_PROTOCOL env var for the app that is being started with dapr run. As explained in the PR in dapr/dapr, this is helpful for the app to know if it should start a server with HTTPS or H2C enabled.

Signed-off-by: ItalyPaleAle <[email protected]>
case "":
return string("http")
default:
return ""
Copy link
Member

@shubham1172 shubham1172 Jun 29, 2023

Choose a reason for hiding this comment

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

Should we error out if appPort is invalid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't normally error in the injector, because that causes the injection to fail and it's hard to debug. I'd let daprd handle the invalid value on startup

@dapr-bot
Copy link
Collaborator

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@dapr-bot dapr-bot added the stale label Jul 29, 2023
@codecov
Copy link

codecov bot commented Jul 29, 2023

Codecov Report

Merging #1318 (6c2b992) into master (31b9ea2) will increase coverage by 0.74%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master    #1318      +/-   ##
==========================================
+ Coverage   26.81%   27.55%   +0.74%     
==========================================
  Files          39       39              
  Lines        3882     3912      +30     
==========================================
+ Hits         1041     1078      +37     
+ Misses       2767     2757      -10     
- Partials       74       77       +3     
Files Changed Coverage Δ
pkg/standalone/run.go 12.54% <83.33%> (+12.54%) ⬆️

@dapr-bot
Copy link
Collaborator

dapr-bot commented Aug 5, 2023

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@dapr-bot dapr-bot closed this Aug 5, 2023
@ItalyPaleAle
Copy link
Contributor Author

Can this be re-opened please? Also, is there something blocking this PR from being merged? @dapr/maintainers-cli @dapr/approvers-cli

@mukundansundar mukundansundar added this to the v1.12 milestone Aug 7, 2023
@mukundansundar mukundansundar merged commit 650fd6f into dapr:master Aug 7, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants