lizhimins commented on PR #1039: URL: https://github.com/apache/rocketmq-dashboard/pull/1039#issuecomment-5249875835
Review update (2026-08-11): this PR builds and its tests pass, but the design now conflicts with the multi-vendor architecture already merged into `rocketmq-studio`, so we cannot merge it as-is. Keeping it open for a rework. **Conflicts with merged work** 1. `cluster/cloud.CloudMetadataProvider` duplicates the existing `provider.CloudCatalogProvider` (listInstances ≡ listCloudInstances, endpoints ≡ CloudInstanceDetailVO); the new `CloudVendor` enum duplicates `InstanceVendor`; the 3 new VOs duplicate CloudRegionVO / CloudInstanceOptionVO / CloudInstanceDetailVO. 2. Credential model: the merged design resolves credentials by `credentialId` from the DB credential store (RmqCloudCredential, see #1147/#1231); this PR switches to global AK/SK environment variables — two mechanisms cannot coexist. 3. SDK split: merged Aliyun integration uses rocketmq20220801 (5.x control plane); this PR adds ons20190214 (4.x ONS), which cannot see 5.x instances, and its client factory duplicates AliyunClientFactory. 4. Direction vs #1535: Tencent capabilities have landed on the InstanceProvider side (TencentInstanceProvider); the TENCENT/HUAWEI enum entries here have no implementation, and `/api/cloud/*` forms a second discovery API alongside the existing catalog controller. **Suggested direction**: rewrite on top of the existing `provider/` stack — extend `CloudCatalogProvider` + DB credential store + the 20220801 SDK instead of introducing a parallel package. Happy to review the rework here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
