-
Notifications
You must be signed in to change notification settings - Fork 202
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 --header/-H to dapr invoke #1287
Conversation
Signed-off-by: hunter007 <[email protected]>
Signed-off-by: hunter007 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hunter007 Thanks for adding this feature.
Can you add E2E for the this feature please?
Signed-off-by: hunter007 <[email protected]>
:) Review this PR first: dapr/go-sdk#405 |
seems the PR has been merged :) |
Yes, so the PR could be reviewd. |
Won't the dependency need to be updated to point to the latest go-sdk in this case? |
no need.
No need. dapr/go-sdk#405 is used when writing app with go-sdk. Without dapr/go-sdk#405, |
@hunter007 the specific test that you have written seems to fail |
It dependent on dapr/go-sdk which's version is after v1.7.0. |
This PR needs to be modified to use the latest go-sdk then in tests. |
@hunter007 1.8 version of go-sdk was released and I believe that needs to be used for the header functionality to work. |
Signed-off-by: hunter007 <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1287 +/- ##
==========================================
+ Coverage 26.81% 26.87% +0.05%
==========================================
Files 39 39
Lines 3882 3885 +3
==========================================
+ Hits 1041 1044 +3
Misses 2767 2767
Partials 74 74
|
@mukundansundar github action timeouted, have a retry? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small changes please.
cmd/invoke.go
Outdated
@@ -98,6 +121,7 @@ func init() { | |||
InvokeCmd.Flags().StringVarP(&invokeData, "data", "d", "", "The JSON serialized data string (optional)") | |||
InvokeCmd.Flags().StringVarP(&invokeVerb, "verb", "v", defaultHTTPVerb, "The HTTP verb to use") | |||
InvokeCmd.Flags().StringVarP(&invokeDataFile, "data-file", "f", "", "A file containing the JSON serialized data (optional)") | |||
InvokeCmd.Flags().StringArrayVarP(&invokeHeaders, "header", "H", []string{}, "0 or more HTTP Header") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InvokeCmd.Flags().StringArrayVarP(&invokeHeaders, "header", "H", []string{}, "0 or more HTTP Header") | |
InvokeCmd.Flags().StringArrayVarP(&invokeHeaders, "header", "H", []string{}, "HTTP headers to be used on invoke") |
resp: "successful invoke", | ||
}, | ||
{ | ||
name: "appID found successful invoke with customized header", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add one test with multiple header values.
Signed-off-by: hunter007 <[email protected]>
Description
to support pass customized headers to dapr app
See dapr/go-sdk#405
Issue reference
Fix #1286