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.

Sign up for GitHub

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

Jump to bottom

feat: support discover grpc pluggable component #991

Merged
merged 19 commits into from
Oct 27, 2023
Merged

feat: support discover grpc pluggable component #991

merged 19 commits into from
Oct 27, 2023

Conversation

cyb0225
Copy link
Contributor

@cyb0225 cyb0225 commented Sep 17, 2023

What this PR does:
layotto 支持跨语言注册 component 功能

Which issue(s) this PR fixes: #959

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

@github-actions
Copy link

Hi @cyb0225. Thanks for your PR! 🎉
If the PR is ready, use the /cc command to assign reviewer to review.

Details

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

The pull request process is described here.

@cyb0225 cyb0225 changed the title [WIP]feat: support discover grpc pluggable component feat: support discover grpc pluggable component Sep 17, 2023
@layotto-cla layotto-cla bot added size/XL and removed size/L labels Sep 18, 2023
@codecov
Copy link

codecov bot commented Sep 19, 2023 •
edited
Loading

Codecov Report

Attention: 115 lines in your changes are missing coverage. Please review.

Files Coverage Δ
components/custom/registry.go 84.00% <100.00%> (ø)
components/hello/helloworld/helloworld.go 100.00% <100.00%> (ø)
components/pluggable/config.go 100.00% <100.00%> (ø)
components/pluggable/register.go 100.00% <100.00%> (ø)
pkg/runtime/secretstores/factory.go 100.00% <100.00%> (ø)
pkg/runtime/secretstores/registry.go 100.00% <100.00%> (ø)
pkg/common/file.go 55.55% <50.00%> (-4.45%) ⬇️
pkg/runtime/options.go 43.33% <50.00%> (ø)
components/hello/pluggable.go 87.09% <87.09%> (ø)
components/pluggable/grpc.go 61.53% <61.53%> (ø)
... and 3 more

📢 Thoughts on this report? Let us know!.

@layotto-cla layotto-cla bot added size/XXL and removed size/XL labels Sep 30, 2023
@zhenjunMa zhenjunMa mentioned this pull request Oct 11, 2023
Closed
@wenxuwan wenxuwan self-requested a review October 13, 2023 09:43
@wenxuwan
Copy link
Member

@cyb0225 先把流水线的错误修复一下，然后加一下quickstart的文档吧

@cyb0225
Copy link
Contributor Author

cyb0225 commented Oct 20, 2023

@cyb0225 先把流水线的错误修复一下，然后加一下quickstart的文档吧

ok

@cyb0225
Copy link
Contributor Author

cyb0225 commented Oct 20, 2023

@wenxuwan done

这两天我研究一下怎么把新 proto 兼容进 makefile proto 生成里，然后设计文档和贡献文档写了

wenxuwan
wenxuwan reviewed Oct 23, 2023
View reviewed changes
pkg/runtime/pluggable/common/config.go Outdated Show resolved Hide resolved
docs/zh/design/pluggable/usage.md Show resolved Hide resolved
pkg/runtime/runtime.go Outdated Show resolved Hide resolved
@zhenjunMa zhenjunMa mentioned this pull request Oct 25, 2023
Closed
wenxuwan
wenxuwan previously approved these changes Oct 26, 2023
View reviewed changes
pkg/runtime/runtime.go Outdated Show resolved Hide resolved
pkg/runtime/pluggable/discover.go Outdated Show resolved Hide resolved
demo/pluggable/hello/main.go Show resolved Hide resolved
docs/zh/design/pluggable/usage.md Outdated Show resolved Hide resolved
wenxuwan
wenxuwan reviewed Oct 26, 2023
View reviewed changes
Copy link
Member

@wenxuwan wenxuwan left a comment

Choose a reason for hiding this comment

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

后续可以考虑一下如果pluggable的组件单独升级，layotto有什么办法能感知到并且切换到新的socket，不用做重启操作

@wenxuwan wenxuwan self-requested a review October 26, 2023 06:04
@cyb0225 cyb0225 force-pushed the feat-pluggable_component branch from 9e57943 to 3ff0b08 Compare October 26, 2023 15:36
@cyb0225
Copy link
Contributor Author

cyb0225 commented Oct 26, 2023

@wenxuwan makefile proto 生成 code 和 doc 的我暂时没调试出来，我打算先写一个doc提供手动编译的方式，再补上 java 的 demo code

@wenxuwan
Copy link
Member

@wenxuwan makefile proto 生成 code 和 doc 的我暂时没调试出来，我打算先写一个doc提供手动编译的方式，再补上 java 的 demo code

好的

@wenxuwan
Copy link
Member

/lgtm

@github-actions
Copy link

[LGTMNOTIFIER] This PR is LGTM

This pull-request has been lgtm by: @wenxuwan

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

The pull request process is described here.

Details

Needs reviewers from an reviewer in each of these files:

Reviewers can indicate their approval by writing /lgtm in a comment
Reviewers can cancel approval by writing /lgtm cancel in a comment

@github-actions github-actions bot added the lgtm label Oct 27, 2023
@wenxuwan
Copy link
Member

/approve

@github-actions
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: @wenxuwan

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

The pull request process is described here.

Details

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

@github-actions github-actions bot merged commit 8f73577 into mosn:main Oct 27, 2023
35 checks passed
@wenxuwan
Copy link
Member

@cyb0225 感谢亦昺的贡献

@zhenjunMa zhenjunMa mentioned this pull request Nov 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

@wenxuwan wenxuwan Awaiting requested review from wenxuwan

Assignees

@wenxuwan wenxuwan

Labels
approved cla:yes lgtm size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cyb0225 @wenxuwan @zhenjunMa