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

Replace gRPC code generation tool from Znly/protoc to Buf #2277

Closed
wants to merge 2 commits into from
Closed

Replace gRPC code generation tool from Znly/protoc to Buf #2277

wants to merge 2 commits into from

Conversation

shivas1516
Copy link

@shivas1516 shivas1516 commented Mar 10, 2024

This commit updates the codebase to utilize Buf for generating gRPC code instead of Zync.

What this PR does / why we need it:

This PR replaces the outdated code generation tool 'znly/protoc' with Buf for generating gRPC code. The change is necessary due to the lack of maintenance and updates for 'znly/protoc', ensuring compatibility, security, and access to new features with Buf.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2141

Checklist:

  • Docs included if any changes are user facing

Copy link

google-cla bot commented Mar 10, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shivas1516
Once this PR has been reviewed and has the lgtm label, please assign andreyvelich for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tenzen-y
Copy link
Member

@shivas1516 Thank you for this contribution!
First of all, could you sign cla and DCO?

@shivas1516
Copy link
Author

Of course! I'm glad you found my contribution valuable. Before proceeding, may I ask if you could kindly provide more context or details regarding the CLA and DCO you mentioned? That way, I can better assist you with the signing process.

@tenzen-y
Copy link
Member

Of course! I'm glad you found my contribution valuable. Before proceeding, may I ask if you could kindly provide more context or details regarding the CLA and DCO you mentioned? That way, I can better assist you with the signing process.

Sure.

CLA singing step: https://github.com/kubeflow/katib/pull/2277/checks?check_run_id=22474756817
DCO signing step: https://github.com/kubeflow/katib/pull/2277/checks?check_run_id=22474756788

This commit updates the codebase to utilize Buf for generating gRPC code instead of Zync.

Signed-off-by: shivas1516 <sivasubramaniaml.ai2021@dce.edu.in>
@shivas1516
Copy link
Author

shivas1516 commented Mar 10, 2024

Of course! I'm glad you found my contribution valuable. Before proceeding, may I ask if you could kindly provide more context or details regarding the CLA and DCO you mentioned? That way, I can better assist you with the signing process.

Sure.

CLA singing step: https://github.com/kubeflow/katib/pull/2277/checks?check_run_id=22474756817 DCO signing step: https://github.com/kubeflow/katib/pull/2277/checks?check_run_id=22474756788

I really appreciate your time and patience. Just to let you know, I've signed both the CLA and the DCO. Thanks again for your help! Looking forward for any suggestions you might have for this PR

@tenzen-y
Copy link
Member

@kubeflow/wg-automl-leads
Could you approve CI? Checking the result of CI would be helpful before we review this.

@tenzen-y
Copy link
Member

@shivas1516 Could you address CI errors?

@shivas1516
Copy link
Author

@shivas1516 Could you address CI errors?

Ofcourse, Let me rework on this CI errors. If any Suggestions regarding this CI Errors please let me know, That will help me to move forward in a correct way faster.

Copy link
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @shivas1516 🎉
Did we investigate what tools other Kubeflow components are using to generate protobufs ?
For example, Kubeflow Pipelines ?

pkg/apis/manager/v1beta1/build.sh Show resolved Hide resolved
@shivas1516
Copy link
Author

Thank you for your contribution @shivas1516 🎉 Did we investigate what tools other Kubeflow components are using to generate protobufs ? For example, Kubeflow Pipelines ?

@andreyvelich Unfortunately, I'm just getting back to you now. I've been tied up addressing the CI errors in this PR. However, I'm having difficulty grasping your thoughts clearly. Could you please provide some detailed explanation for better clarity?

@shivas1516 shivas1516 requested a review from andreyvelich March 15, 2024 10:02
@shivas1516 shivas1516 changed the title Replace gRPC code generation tool from Zync to Buf Replace gRPC code generation tool from Znly/protoc to Buf Mar 15, 2024
@andreyvelich
Copy link
Member

andreyvelich commented Mar 16, 2024

Thank you for your contribution @shivas1516 🎉 Did we investigate what tools other Kubeflow components are using to generate protobufs ? For example, Kubeflow Pipelines ?

@andreyvelich Unfortunately, I'm just getting back to you now. I've been tied up addressing the CI errors in this PR. However, I'm having difficulty grasping your thoughts clearly. Could you please provide some detailed explanation for better clarity?

@shivas1516 Currently, if we make any changes in api.proto, we can run make generate to re-generate files. Since you are going to remove pkg/apis/manager/health/build.sh file, how we can re-generate gRPC files ?

@shivas1516
Copy link
Author

shivas1516 commented Mar 16, 2024

Thank you for your contribution @shivas1516 🎉 Did we investigate what tools other Kubeflow components are using to generate protobufs ? For example, Kubeflow Pipelines ?

@andreyvelich Unfortunately, I'm just getting back to you now. I've been tied up addressing the CI errors in this PR. However, I'm having difficulty grasping your thoughts clearly. Could you please provide some detailed explanation for better clarity?

@shivas1516 Currently, if we make any changes in api.proto, we can run make generate to re-generate files. Since you are going to remove pkg/apis/manager/health/build.sh file, how we can re-generate gRPC files ?

@andreyvelich When migrating to Buf, instead of using make generate, we must utilize the buf generate command to regenerate code changes in .proto files. This command relies on configurations specified in buf.gen.yaml, where we can define language preferences for generating code, remote plugins to use, and versions for our convenience.

Hope I'm clear to you, If any suggestions or need clarity regarding this. Please let me know.

@shivas1516 shivas1516 closed this by deleting the head repository Mar 24, 2024
@tenzen-y
Copy link
Member

tenzen-y commented Mar 25, 2024

When migrating to Buf, instead of using make generate, we must utilize the buf generate command to regenerate code changes in .proto files. This command relies on configurations specified in buf.gen.yaml, where we can define language preferences for generating code, remote plugins to use, and versions for our convenience.

Hope I'm clear to you, If any suggestions or need clarity regarding this. Please let me know.

@shivas1516 Any reasons that you closed this PR?

@shivas1516
Copy link
Author

When migrating to Buf, instead of using make generate, we must utilize the buf generate command to regenerate code changes in .proto files. This command relies on configurations specified in buf.gen.yaml, where we can define language preferences for generating code, remote plugins to use, and versions for our convenience.

Hope I'm clear to you, If any suggestions or need clarity regarding this. Please let me know.

@shivas1516 Any reasons that you closed this PR?

Thank you for your inquiry regarding the closure of the PR.

Unfortunately, the PR was closed due to an error that occurred in the my forked Katib repository GitHub codespace. Regrettably, I had to delete the forked repository master as a result. However, I am currently reworking on this issue, and I am confident that it will be even better this time around. I have reached out to the Buf community and received valuable suggestions from them, which I believe will contribute to the improvement of the project.

@tenzen-y
Copy link
Member

When migrating to Buf, instead of using make generate, we must utilize the buf generate command to regenerate code changes in .proto files. This command relies on configurations specified in buf.gen.yaml, where we can define language preferences for generating code, remote plugins to use, and versions for our convenience.
Hope I'm clear to you, If any suggestions or need clarity regarding this. Please let me know.

@shivas1516 Any reasons that you closed this PR?

Thank you for your inquiry regarding the closure of the PR.

Unfortunately, the PR was closed due to an error that occurred in the my forked Katib repository GitHub codespace. Regrettably, I had to delete the forked repository master as a result. However, I am currently reworking on this issue, and I am confident that it will be even better this time around. I have reached out to the Buf community and received valuable suggestions from them, which I believe will contribute to the improvement of the project.

Thank you for your effort. I just wanted to confirm if you're still in here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace the tool of code generator for the gRPC API
3 participants