Added internal-grpc-port to standalone run options #876
Conversation
Signed-off-by: Jigar R Gandhi <jigarr.gandhi@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #876 +/- ##
=======================================
Coverage 29.36% 29.36%
=======================================
Files 35 35
Lines 2329 2329
=======================================
Hits 684 684
Misses 1572 1572
Partials 73 73
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@yaron2 Can you review this PR? |
|
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! |
|
@yaron2 please review this |
|
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! |
|
@jigargandhi Please resolve the conflicts in the PR ... cc @yaron2 once conflicts are resolved, please review the PR |
|
@yaron2 please review the PR, @mukundansundar I have resolved the merge conflicts |
|
@jigargandhi Can you resolve the conflicts for this? |
|
@jigargandhi Can you resolve the conflicts for this PR? |
|
@mukundansundar resolved the conflicts |
|
@pravinpushkar Please review this PR .. |
|
@jigargandhi FYI - The related PRs in the description are not merged and probably got closed due to no activity. |
|
@jigargandhi Does it make sense if we expose this as configurable option (get free port only when not provided by cmd line)? Another clarification, if this is supposed to be only internal then should it be part of cli ? |
There was a problem hiding this comment.
@jigargandhi Thanks for the contribution!
Also please do add E2E for this.
| @@ -372,3 +379,18 @@ func init() { | |||
|
|
|||
| RootCmd.AddCommand(RunCmd) | |||
| } | |||
|
|
|||
| // GetFreePort asks the kernel for a free open port that is ready to use. | |||
| func getFreePort() (int, error) { | |||
There was a problem hiding this comment.
@jigargandhi we are already importing "github.com/phayes/freeport" in the pkg/standalone/run.go file. Why can't we use that here?
| @@ -99,6 +100,11 @@ dapr run --app-id myapp --app-port 3000 --app-protocol grpc -- go run main.go | |||
| } | |||
| } | |||
|
|
|||
| internalGrpcPort, err := getFreePort() | |||
There was a problem hiding this comment.
Can you also change the implementation to allow passing in internal grpc port from cmd flag? Wrt issue #1026
|
I am not planning to work on this at the moment, please feel free to take it up. |
|
I can take this PR to completion, will cherry pick commits and create a new PR. |
|
closed by #1040 |
Signed-off-by: Jigar R Gandhi jigarr.gandhi@gmail.com
Description
Added dapr-internal-grpc-port argument to daprd process command line for standalone.
This value will be used when parsing the value from a local DNS component which is implemented as per this
dapr/components-contrib#1340.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR related to: dapr/dapr#3256
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: