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

feat: support nacos configstore component (level1) #939

Merged
merged 26 commits into from
Jun 30, 2023

Conversation

cyb0225
Copy link
Contributor

@cyb0225 cyb0225 commented Jun 7, 2023

What this PR does:

support nacos configstore component with level1.

Which issue(s) this PR fixes: #921

Fixes #

Special notes for your reviewer:

参考文档: https://mosn.io/layotto/#/zh/design/configuration/configuration-api-with-apollo

目前的 mock 采用的是 gomock 生成,并未实际模拟 nacos 配置中心的行为,只是对测试输入和结果进行预定义,只针对个人编写代码进行了测试(比如参数校验,分支预测等)

情况完成

完成:

  • level1 模式的配置中心功能
  • 单元测试
  • 完善连接 nacos 配置的方式。(支持连接自建 nacos 服务,以及连接阿里云 nacos 服务)
  • 分页操作。

未完成:

  • 相关文档

Does this PR introduce a user-facing change?:


@cyb0225 cyb0225 marked this pull request as draft June 7, 2023 16:53
@github-actions
Copy link

github-actions bot commented Jun 7, 2023

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.

@wenxuwan wenxuwan self-requested a review June 9, 2023 07:37
@cyb0225 cyb0225 changed the title feat: support nacos configstore component (level1) feat: support nacos configstore component (level2) Jun 10, 2023
@cyb0225 cyb0225 force-pushed the component-nacos branch 2 times, most recently from 1160a1f to 82ff44c Compare June 15, 2023 07:10
@cyb0225 cyb0225 changed the title feat: support nacos configstore component (level2) feat: support nacos configstore component (level1) Jun 15, 2023
support level1 config functions.
support connect to nacos use acm mode.
fix error in filling in illegal characters when calling nacos set and delete parameters
@cyb0225 cyb0225 marked this pull request as ready for review June 16, 2023 04:27
@cyb0225
Copy link
Contributor Author

cyb0225 commented Jun 16, 2023

@wenxuwan, 大致功能已经做完了,你看看。还剩下几个地方可能需要再讨论一下。

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Patch coverage: 79.04% and project coverage change: +0.65 🎉

Comparison is base (8f2c52f) 59.80% compared to head (e16a6d4) 60.45%.

❗ Current head e16a6d4 differs from pull request most recent head aa6d958. Consider uploading reports for the commit aa6d958 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #939      +/-   ##
==========================================
+ Coverage   59.80%   60.45%   +0.65%     
==========================================
  Files         138      142       +4     
  Lines        9546     9880     +334     
==========================================
+ Hits         5709     5973     +264     
- Misses       3116     3160      +44     
- Partials      721      747      +26     
Impacted Files Coverage Δ
components/configstores/nacos/config.go 60.60% <60.60%> (ø)
components/configstores/nacos/configstore.go 79.42% <79.42%> (ø)
components/configstores/nacos/default_logger.go 100.00% <100.00%> (ø)
components/configstores/nacos/errors.go 100.00% <100.00%> (ø)
pkg/runtime/runtime.go 54.59% <100.00%> (+0.11%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

components/configstores/nacos/configstore.go Show resolved Hide resolved
components/configstores/nacos/const.go Outdated Show resolved Hide resolved
components/configstores/nacos/config.go Outdated Show resolved Hide resolved
components/configstores/nacos/config.go Show resolved Hide resolved
components/configstores/nacos/configstore.go Outdated Show resolved Hide resolved
components/configstores/nacos/configstore.go Outdated Show resolved Hide resolved
components/configstores/nacos/default_logger.go Outdated Show resolved Hide resolved
components/configstores/nacos/configstore.go Outdated Show resolved Hide resolved
components/configstores/nacos/configstore.go Outdated Show resolved Hide resolved
@wenxuwan
Copy link
Member

@cyb0225 reviewer的cr的comments如果修复了,可以在comments下面回复个done之类的,然后reviewer可以继续cr,如果没问题了,reviewer才可以点
image

@wenxuwan
Copy link
Member

根据这个 https://mosn.io/layotto/#/zh/development/developing-component 文档,写个demo和快速开始文档再,🌹

@cyb0225
Copy link
Contributor Author

cyb0225 commented Jun 29, 2023

根据这个 https://mosn.io/layotto/#/zh/development/developing-component 文档,写个demo和快速开始文档再,🌹

这个 ci 错误感觉可以忽略,因为我在代码里提前写了 nacos config 的路径,不过还没 merge 进去所以 deadlink 这个 ci 没跑通。

quick start 我打算 merge 进去后,写一个 docker-compose,用新版的 layotto 重新提一个 pr,你看看怎么样。

@wenxuwan
Copy link
Member

quick start 我打算 merge 进去后,写一个 docker-compose,用新版的 layotto 重新提一个 pr,你看看怎么样。

嗯,是可以忽略

@wenxuwan wenxuwan self-requested a review June 30, 2023 03:13
@wenxuwan wenxuwan enabled auto-merge (squash) June 30, 2023 03:13
@wenxuwan wenxuwan merged commit 2354cd7 into mosn:main Jun 30, 2023
24 of 25 checks passed
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.

LGTM, @cyb0225 感谢贡献

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

Successfully merging this pull request may close these issues.

2 participants