sandynz commented on PR #2527:
URL:
https://github.com/apache/shardingsphere-elasticjob/pull/2527#issuecomment-5101868893
### Summary
**Review Result: Mergeable**
**Reason:** The change fixes the stale job-configuration read at the
lifecycle boundary without changing kernel cache behavior or public contracts.
This result is code-scope only; CI was not reviewed by request.
### Evidence
- `ZookeeperRegistryCenter#get()` reads from `CuratorCache` when the key
belongs to a cached job path, while `getDirectly()` reads through the registry
client (`registry-center/provider/zookeeper-curator/.../
ZookeeperRegistryCenter.java:159-180`).
- Kernel job registration still creates the per-job cache through
`JobRegistry#registerRegistryCenter()` (`kernel/.../JobRegistry.java:70-72`).
- `JobConfigurationAPIImpl#getJobConfiguration()` now obtains the
configuration directly from the registry center
(`lifecycle/.../JobConfigurationAPIImpl.java:38-43`).
- `JobStatisticsAPIImpl#getJobBriefInfo()` also reads the configuration
directly, while existing server, sharding, and instance state reads retain
their previous behavior (`lifecycle/.../JobStatisticsAPIImpl.java:65-
78,97-116`).
- The existing lifecycle tests now explicitly require `getDirectly()`,
covering missing configurations, supported job configuration types, individual
job summaries, status branches, and all-job listing
(`JobConfigurationAPIImplTest.java:54-77`,
`JobStatisticsAPIImplTest.java:57-130`).
- The change does not modify public API/SPI signatures, configuration
formats, dependencies, registry-center cache lifecycle, or distribution
metadata.
### Review Details
- **Review Focus:** Code Correctness Review. CI not reviewed by request.
- **Reviewed Scope:** Commit `5f0cf5da25b5ec16a45094de0c087326d9ced413`;
local merge-base `1a01674b86ee283b102d495ce077baa0fe248a70`; all four changed
files in the lifecycle module were reviewed. No GitHub PR exists
yet, so the local file list could not be compared with
`/pulls/{number}/files`.
- **Not Reviewed Scope:** GitHub PR metadata, review discussions, GitHub
Actions, and cross-project consumer integration.
- **Verification:**
- `./mvnw -pl lifecycle
-Dtest=JobConfigurationAPIImplTest,JobStatisticsAPIImplTest test -Pcheck -T1C`
— exit 0; 17 tests, 0 failures, 0 errors.
- `./mvnw spotless:check checkstyle:check apache-rat:check -Pcheck -T1C`
— exit 0; build successful with 0 Checkstyle violations.
- **Release Note / User Docs:** Not required. This is an internal
lifecycle consistency fix with no user-facing API, configuration, migration, or
compatibility change.
--
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]
