Skip to content

refactor: remove config package from polaris and config_center#3210

Open
zbchi wants to merge 10 commits intoapache:developfrom
zbchi:replace-polaris-config
Open

refactor: remove config package from polaris and config_center#3210
zbchi wants to merge 10 commits intoapache:developfrom
zbchi:replace-polaris-config

Conversation

@zbchi
Copy link
Contributor

@zbchi zbchi commented Feb 11, 2026

Description

Remove dependency on config package from polaris limiter and config_center modules, use URL parameters/attributes and global package types instead.

Checklist

  • I confirm the target branch is develop
  • Code has passed local testing
  • I have added tests that prove my fix is effective or that my feature works

@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.91%. Comparing base (35ea886) to head (d29a6de).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
config_center/zookeeper/impl.go 0.00% 3 Missing ⚠️
filter/polaris/limit/limiter.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3210      +/-   ##
===========================================
- Coverage    47.99%   47.91%   -0.09%     
===========================================
  Files          463      463              
  Lines        33727    33744      +17     
===========================================
- Hits         16187    16168      -19     
- Misses       16237    16271      +34     
- Partials      1303     1305       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlexStocks AlexStocks added the 3.3.2 version 3.3.2 label Feb 11, 2026
@AlexStocks AlexStocks requested review from Alanxtl and Copilot and removed request for Copilot February 11, 2026 13:19
Copy link
Contributor

@nanjiek nanjiek left a comment

Choose a reason for hiding this comment

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

common/constant/key里面有常量了,测试直接复用就好了。

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors Polaris limiter and config center Apollo tests to avoid depending on the config package, instead deriving configuration from URL params/attributes and global types.

Changes:

  • Replace config lookups in the Polaris TPS limiter with URL attribute/parameter based detection.
  • Update Apollo config center tests to unmarshal into global.ApplicationConfig and construct config-center URL params directly.
  • Minor cleanup in Zookeeper config center initialization struct literal.

Reviewed changes

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

File Description
filter/polaris/limit/limiter.go Switches Polaris limiter “applicationMode” detection from global config to URL attributes/params.
config_center/zookeeper/impl.go Minor struct literal cleanup in Zookeeper dynamic configuration constructor.
config_center/apollo/impl_test.go Removes config.RootConfig usage in tests; uses global.ApplicationConfig and URL params.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

zbchi and others added 5 commits February 11, 2026 23:01
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

@nanjiek nanjiek left a comment

Choose a reason for hiding this comment

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

还有未替换成常量的固定值

@Alanxtl Alanxtl linked an issue Feb 12, 2026 that may be closed by this pull request
@sonarqubecloud
Copy link

Copy link
Contributor

@Alanxtl Alanxtl left a comment

Choose a reason for hiding this comment

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

lgtm

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.

[Enhancement] Complete delete config package

6 participants