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: read config from nacos #838

Merged
merged 2 commits into from
May 9, 2023
Merged

feat: read config from nacos #838

merged 2 commits into from
May 9, 2023

Conversation

5idu
Copy link
Contributor

@5idu 5idu commented Apr 24, 2023

Describe what this PR does / why we need it

support read config from nacos datasource.

Does this pull request fix one issue?

Describe how to verify it

go run main.go --config="nacos://ip:port/config.yaml?dataId=mytest&group=xxx&namespaceId=xxx&timeout=10000&accessKey=xxx&secretKey=xxx"

@5idu
Copy link
Contributor Author

5idu commented Apr 27, 2023

@hnlq715 can help me review the code, tks.

@sysulq
Copy link
Member

sysulq commented Apr 28, 2023

Thanks for contribution, please pass golangci-lint and add some tests to this nacos implementation.

@5idu 5idu temporarily deployed to tablestore-live May 5, 2023 05:10 — with GitHub Actions Inactive
@codecov-commenter
Copy link

codecov-commenter commented May 5, 2023

Codecov Report

Merging #838 (daefe56) into master (521af59) will increase coverage by 0.04%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #838      +/-   ##
==========================================
+ Coverage   52.87%   52.91%   +0.04%     
==========================================
  Files         202      202              
  Lines       11318    11326       +8     
==========================================
+ Hits         5984     5993       +9     
+ Misses       4879     4877       -2     
- Partials      455      456       +1     
Impacted Files Coverage Δ
pkg/util/xnet/url.go 0.00% <0.00%> (ø)

... and 4 files with indirect coverage changes

@5idu
Copy link
Contributor Author

5idu commented May 5, 2023

@hnlq715 has modified, please review again.

@sysulq sysulq merged commit fc7ad03 into douyu:master May 9, 2023
sysulq added a commit that referenced this pull request May 9, 2023
sysulq added a commit that referenced this pull request May 9, 2023
Revert "feat: read config from nacos (#838)"

This reverts commit fc7ad03.
@5idu
Copy link
Contributor Author

5idu commented May 11, 2023

@hnlq715 I saw that this PR was reverted, is there any problem?

@sysulq
Copy link
Member

sysulq commented May 15, 2023

@5idu
Seems like the unit test case has failed, and we need publish a new release, so...We need to solve this data race problem before merge this PR.

https://github.com/douyu/jupiter/actions/runs/4921729959/jobs/8791856738

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.

None yet

3 participants