Skip to content
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

[FEATURE] - Implement GRPC executor #1049

Merged
merged 3 commits into from
Apr 24, 2022

Conversation

Espina2
Copy link
Contributor

@Espina2 Espina2 commented Jan 22, 2022

Implement GRPC executor relaying in GRPC server reflection to get proto descriptors and serialize protobufs from JSON.

Out of scope:

  • Supporting other GRPC encoding types

Signed-off-by: Paulo Moura itsme@paulomoura.com.pt

@Espina2 Espina2 marked this pull request as draft January 22, 2022 18:03
@Espina2 Espina2 marked this pull request as ready for review January 22, 2022 18:40
@Espina2
Copy link
Contributor Author

Espina2 commented Jan 22, 2022

@vcastellm can you take a look?

@Espina2
Copy link
Contributor Author

Espina2 commented Jan 30, 2022

@vcastellm did you have the opportunity to take a look?

Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM overall but I'm missing documentation and some example use cases for this.

@Espina2
Copy link
Contributor Author

Espina2 commented Feb 13, 2022

LGTM overall but I'm missing documentation and some example use cases for this.

I added usage documentation. Can you point me in the direction that you pretend?

@Espina2
Copy link
Contributor Author

Espina2 commented Mar 13, 2022

@vcastellm I would love to merge this. Also is it possible to release after merge?

vcastellm
vcastellm previously approved these changes Mar 16, 2022
Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM aside from the typo

website/content/usage/executors/grpc.md Outdated Show resolved Hide resolved
Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

Can you rebase with master?

Signed-off-by: Paulo Moura <itsme@paulomoura.com.pt>
Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

Thanks @Espina2

@vcastellm vcastellm merged commit f0c0df3 into distribworks:master Apr 24, 2022
@Espina2 Espina2 deleted the grpc-executor branch April 24, 2022 22:35
@Espina2
Copy link
Contributor Author

Espina2 commented Apr 24, 2022

@vcastellm any way that we can make a release of this?

@vcastellm
Copy link
Member

@Espina2 It will be included in the next release

@Espina2
Copy link
Contributor Author

Espina2 commented Apr 26, 2022

@Espina2 It will be included in the next release

When will that happen?

@vcastellm
Copy link
Member

@Espina2 I want to release v3.2.0 in the following days.

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.

2 participants