Skip to content

Conversation

@patskovn
Copy link
Contributor

Currently interceptors does not propagated to calls. Seems like it was forgotten to be implemented in this alpha implementation.

Currently interceptors does not propagated to calls
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 19, 2021

CLA Signed

The committers are authorized under a signed CLA.

@Lukasa
Copy link
Collaborator

Lukasa commented Sep 20, 2021

Thanks for this! Can you regenerate the various grpc examples? make generate-echo generate-helloworld generate-route-guide generate-normalization should do it.

@patskovn
Copy link
Contributor Author

Thanks for this! Can you regenerate the various grpc examples? make generate-echo generate-helloworld generate-route-guide generate-normalization should do it.

Done. Async flag set only for echo generation, but I still launch whole generation. It added additional newline to the bottom of files. Not sure is it ok or not :)

@Lukasa
Copy link
Collaborator

Lukasa commented Sep 20, 2021

The extra newlines are fine 😄

@Lukasa
Copy link
Collaborator

Lukasa commented Sep 20, 2021

Do you feel up to writing some tests? I'd love to see a couple of new tests, probably in InterceptorsTests, that validate that the interceptors continue to work correctly in async/await land.

@patskovn
Copy link
Contributor Author

I decided to create separate test class for async interceptors in analogy with other async tests like GRPCAsyncClientCallTests. I called file InterceptorsAsyncTests so that its be close to default counterpart InterceptorsTests.
Also, writing tests directly in current InterceptorsTests file will cause it suffer from lots of ifdefs and @availables

Waiting for further improvements suggestion 👌

@patskovn
Copy link
Contributor Author

Hello, @Lukasa! What are next steps to make further progress in this PR? Should I wait more for your review when you will have time or should I improve it somehow?

@glbrntt glbrntt self-requested a review September 27, 2021 12:56
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Sep 27, 2021
Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Oops..! Thanks for catching and fixing this @Nekitosss. Would you mind running ./scripts/format.sh to make the CI happy?

Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Great, thanks @Nekitosss 🙏

@glbrntt glbrntt merged commit b545bdd into grpc:1.4.1-async-await Sep 29, 2021
glbrntt pushed a commit that referenced this pull request Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants