Skip to content

Refactor protoc to descriptor conversion#1493

Merged
fjtirado merged 3 commits into
serverlessworkflow:mainfrom
mcruzdev:issue-1492
Jun 29, 2026
Merged

Refactor protoc to descriptor conversion#1493
fjtirado merged 3 commits into
serverlessworkflow:mainfrom
mcruzdev:issue-1492

Conversation

@mcruzdev

@mcruzdev mcruzdev commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:

Special notes for reviewers:

Additional information (if needed):

Closes #1492

@mcruzdev
Show a better error message when the .proto descriptor is not available
21b666f 
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
@mcruzdev mcruzdev requested a review from fjtirado as a code owner June 26, 2026 18:15
Copilot AI review requested due to automatic review settings June 26, 2026 18:15
Copilot AI reviewed Jun 26, 2026
View reviewed changes

Copilot AI left a comment •
edited by fjtirado
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

The grpc proto invocation was refactored to simplify exception handling. As a consequence of this, there will be not longer a bare null pointer exception, but a message including the failing grpc path.

Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java Outdated
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java Outdated
@fjtirado
Review comments
3537489 
Signed-off-by: fjtirado <ftirados@ibm.com>
Copilot AI review requested due to automatic review settings June 29, 2026 09:59
Copilot AI reviewed Jun 29, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java Outdated
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/resources/ExternalResourceHandler.java
Comment thread impl/core/src/main/java/io/serverlessworkflow/impl/resources/ClasspathResource.java Outdated
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java Outdated
fjtirado
fjtirado approved these changes Jun 29, 2026
View reviewed changes

@fjtirado fjtirado left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@mcruzdev I took the liberty of using this PR to refactor the way grpc files were handled.
Hope you like it.

fjtirado
fjtirado reviewed Jun 29, 2026
View reviewed changes
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java
e);
} finally {
if (grpcDir != null) {
deleteTempFiles(grpcDir);

@fjtirado fjtirado Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The temporary dir used for protoc execution should be deleted.

@fjtirado fjtirado requested a review from Copilot June 29, 2026 11:09
Copilot AI reviewed Jun 29, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/GrpcExecutor.java Outdated
@fjtirado fjtirado requested a review from Copilot June 29, 2026 11:23
Copilot AI reviewed Jun 29, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java Outdated
Copilot AI review requested due to automatic review settings June 29, 2026 11:26
@fjtirado fjtirado changed the title Show a better error message when the .proto descriptor is not available Refactor protoc to descriptor conversion Jun 29, 2026
Copilot AI reviewed Jun 29, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java Outdated
@fjtirado fjtirado requested a review from Copilot June 29, 2026 11:36
Copilot AI reviewed Jun 29, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/FileDescriptorReader.java
Comment thread impl/grpc/src/main/java/io/serverlessworkflow/impl/executors/grpc/GrpcExecutor.java
@fjtirado fjtirado merged commit 55765c4 into serverlessworkflow:main Jun 29, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fjtirado fjtirado fjtirado approved these changes
Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Show a better message error when the proto descriptor is not available

3 participants

@mcruzdev @fjtirado