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

[ISSUE #4345] Add support for polaris config sync #4410

Merged
merged 91 commits into from
Jul 25, 2023

Conversation

fabian4
Copy link
Member

@fabian4 fabian4 commented Feb 26, 2023

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

TODO:

Related issue: #4345

ShenYu admin

ShenYu bootstarap

  • Add PolarisListener for config changing.
  • Put it in a polaris stater.
  • Add relevant test case.

@fabian4
Copy link
Member Author

fabian4 commented Feb 26, 2023

As for support pushing configuration to Polaris:

The polaris client does not contain a method to publish the config to Polaris.

See as: https://github.com/polarismesh/polaris-java/blob/main/polaris-configuration/polaris-configuration-api/src/main/java/com/tencent/polaris/configuration/api/core/ConfigFileService.java

package org.apache.shenyu.admin.listener.polaris;

@Override
public void publishConfig(String dataId, Object data) {
    LOG.warn("Config upload not support yet, please upload it in polaris first");
}

So if we change on the admin dashboard only, it can not sync to Polaris and the bootstrap application.

@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2023

Codecov Report

Merging #4410 (e49a0b9) into master (3d3cafe) will increase coverage by 0.08%.
The diff coverage is 78.26%.

@@             Coverage Diff              @@
##             master    #4410      +/-   ##
============================================
+ Coverage     63.89%   63.98%   +0.08%     
- Complexity     8235     8291      +56     
============================================
  Files          1169     1177       +8     
  Lines         34541    34725     +184     
  Branches       3107     3111       +4     
============================================
+ Hits          22071    22219     +148     
- Misses        10706    10734      +28     
- Partials       1764     1772       +8     
Files Changed Coverage Δ
...e/shenyu/common/constant/PolarisPathConstants.java 0.00% <0.00%> (ø)
...n/listener/polaris/PolarisDataChangedListener.java 60.00% <60.00%> (ø)
...sync/data/polaris/handler/PolarisCacheHandler.java 75.30% <75.30%> (ø)
...admin/listener/polaris/PolarisDataChangedInit.java 78.57% <78.57%> (ø)
...nyu/admin/config/properties/PolarisProperties.java 80.00% <80.00%> (ø)
...shenyu/sync/data/polaris/config/PolarisConfig.java 80.00% <80.00%> (ø)
...che/shenyu/admin/config/DataSyncConfiguration.java 77.27% <100.00%> (+3.58%) ⬆️
...ync/data/polaris/PolarisSyncDataConfiguration.java 100.00% <100.00%> (ø)
...enyu/sync/data/polaris/PolarisSyncDataService.java 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@damonxue
Copy link
Contributor

As for support pushing configuration to Polaris:

The polaris client does not contain a method to publish the config to Polaris.

See as: https://github.com/polarismesh/polaris-java/blob/main/polaris-configuration/polaris-configuration-api/src/main/java/com/tencent/polaris/configuration/api/core/ConfigFileService.java

package org.apache.shenyu.admin.listener.polaris;

@Override
public void publishConfig(String dataId, Object data) {
    LOG.warn("Config upload not support yet, please upload it in polaris first");
}

So if we change on the admin dashboard only, it can not sync to Polaris and the bootstrap application.

Now, Polaris-java SDK hasn't supported the sync config. But there is the other way to sync.
Refer to config sync codes

@damonxue
Copy link
Contributor

Pls add my WeChat(id: Fibonacci_stack), there is a group to support Polaris.

@fabian4
Copy link
Member Author

fabian4 commented Feb 27, 2023

As for support pushing configuration to Polaris:
The polaris client does not contain a method to publish the config to Polaris.

See as: https://github.com/polarismesh/polaris-java/blob/main/polaris-configuration/polaris-configuration-api/src/main/java/com/tencent/polaris/configuration/api/core/ConfigFileService.java

package org.apache.shenyu.admin.listener.polaris;

@Override
public void publishConfig(String dataId, Object data) {
    LOG.warn("Config upload not support yet, please upload it in polaris first");
}

So if we change on the admin dashboard only, it can not sync to Polaris and the bootstrap application.

Now, Polaris-java SDK hasn't supported the sync config. But there is the other way to sync. Refer to config sync codes

Thanks for that, let me see what I can do.💪

shenyu-bootstrap/pom.xml Outdated Show resolved Hide resolved
@fabian4 fabian4 changed the title [ISSUE #4345] Add support for polaris [ISSUE #4345] Add support for polaris config sync Feb 27, 2023
shenyu-admin/pom.xml Outdated Show resolved Hide resolved
@fabian4
Copy link
Member Author

fabian4 commented Jul 19, 2023

wait com.tencent.polaris:polaris-all have a release version

It is officially released now.
https://github.com/polarismesh/polaris-java/releases

fabian4 and others added 4 commits July 19, 2023 14:29
@loongs-zhang
Copy link
Member

The rest parts looks fine.

@fabian4 fabian4 requested a review from damonxue July 24, 2023 08:01
Copy link
Contributor

@damonxue damonxue left a comment

Choose a reason for hiding this comment

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

pls @mahaitao617 review.

@loongs-zhang loongs-zhang self-requested a review July 25, 2023 06:58
@loongs-zhang loongs-zhang added this to the 2.6.0 milestone Jul 25, 2023
@loongs-zhang loongs-zhang merged commit 39db11b into apache:master Jul 25, 2023
38 checks passed
@fabian4 fabian4 deleted the add_support_for_polaris branch July 25, 2023 07:25
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.

None yet

6 participants