search

dapr / cli

Command-line tools for Dapr.
Apache License 2.0
315 stars 200 forks source link

Add --header/-H to dapr invoke #1287

Closed hunter007 closed 1 year ago

hunter007 commented 1 year ago

Description

to support pass customized headers to dapr app

See https://github.com/dapr/go-sdk/pull/405

Issue reference

Fix https://github.com/dapr/cli/issues/1286

hunter007 commented 1 year ago

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) Review this PR first: https://github.com/dapr/go-sdk/pull/405

mukundansundar commented 1 year ago

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) Review this PR first: dapr/go-sdk#405

seems the PR has been merged :)

hunter007 commented 1 year ago

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) Review this PR first: dapr/go-sdk#405

seems the PR has been merged :)

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) Review this PR first: dapr/go-sdk#405

seems the PR has been merged :)

Yes, so the PR could be reviewd.

mukundansundar commented 1 year ago

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) Review this PR first: dapr/go-sdk#405

seems the PR has been merged :)

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) 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?

hunter007 commented 1 year ago

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) Review this PR first: dapr/go-sdk#405

seems the PR has been merged :)

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) 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.

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) Review this PR first: dapr/go-sdk#405

seems the PR has been merged :)

@hunter007 Thanks for adding this feature. Can you add E2E for the this feature please?

:) 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. https://github.com/dapr/go-sdk/pull/405 is used when writing app with go-sdk. Without https://github.com/dapr/go-sdk/pull/405, cli invoke will still send HTTP header by -H to dapr app

mukundansundar commented 1 year ago

@hunter007 the specific test that you have written seems to fail output does not contain "aValue" ... Can you take a look into this?

hunter007 commented 1 year ago

@hunter007 the specific test that you have written seems to fail output does not contain "aValue" ... Can you take a look into this?

It dependent on dapr/go-sdk which's version is after v1.7.0.

mukundansundar commented 1 year ago

@hunter007 the specific test that you have written seems to fail output does not contain "aValue" ... Can you take a look into this?

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.

mukundansundar commented 1 year ago

@hunter007 1.8 version of go-sdk was released and I believe that needs to be used for the header functionality to work.

codecov[bot] commented 1 year ago

Codecov Report

Merging #1287 (83d8a3a) into master (31b9ea2) will increase coverage by 0.05%. The diff coverage is 100.00%.

@@            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
Impacted Files Coverage Δ
pkg/standalone/client.go 0.00% <ø> (ø)
pkg/standalone/invoke.go 73.33% <100.00%> (+1.90%) :arrow_up:
hunter007 commented 1 year ago

@mukundansundar github action timeouted, have a retry?