Skip to content

Conversation

@artursouza
Copy link
Contributor

Description

Fix bi-di subscription to support dapr-api-token

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 will close: #1072

Checklist

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

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

*/
public ActorClient(Properties overrideProperties) {
this(buildManagedChannel(overrideProperties), null);
this(buildManagedChannel(overrideProperties), null, overrideProperties.getValue(Properties.API_TOKEN));
Copy link
Contributor Author

@artursouza artursouza Oct 10, 2024

Choose a reason for hiding this comment

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

I also needed to fix how properties override work in some cases because it was not handling overrides correctly for Dapr API Token. Now, it works too and all ITs require sidecar calls to have dapr-api-token header.

TypeRef<T> type,
DaprProtos.SubscribeTopicEventsRequestAlpha1 request) {
Subscription<T> subscription = new Subscription<>(this.asyncStub, request, listener, response -> {
var interceptedStub = this.grpcInterceptors.intercept(this.asyncStub);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the fix for streaming subscription to handle Dapr API token header.

@artursouza artursouza marked this pull request as ready for review October 10, 2024 22:57
@artursouza artursouza requested review from a team as code owners October 10, 2024 22:57
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
cicoyle
cicoyle previously approved these changes Oct 15, 2024
@cicoyle
Copy link
Contributor

cicoyle commented Oct 15, 2024

Changes lgtm, but I see some build failures - might be flaky

@artursouza
Copy link
Contributor Author

Changes lgtm, but I see some build failures - might be flaky

I have been fixing those for the past few days. Almost there.

Signed-off-by: Artur Souza <asouza.pro@gmail.com>
@artursouza
Copy link
Contributor Author

Changes lgtm, but I see some build failures - might be flaky

I have been fixing those for the past few days. Almost there.

I think that was the last one. Waiting for the run now.

@artursouza artursouza merged commit 6a6901d into dapr:master Oct 16, 2024
7 checks passed
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.

Add Streaming Subscription Support

2 participants