GGraziadei commented on PR #8819: URL: https://github.com/apache/storm/pull/8819#issuecomment-4946185795
The overall implementation is solid, well-documented, and appropriately conservative. One blocking concern (classpath shadowing). An audit of `bin/storm.py` shows `get_classpath` prepends `lib-common` before `lib` on the daemon classpath. This introduces a silent shadowing risk under future dependency drift: if a same-named jar ever diverges (SHA mismatch), it is not removed from lib, so both copies end up on the classpath and the worker's lib-common copy wins; likewise, any worker-only jar is promoted onto the daemon classpath. The single-reactor build with shared dependencyManagement prevents this today, but it's a fragile, hard-to-debug failure mode.I advice to add: - a safety guard in dedup-libs.py that fails (or loudly warns) on same-name/different-SHA jars, ideally also printing the count of worker-only jars promoted to lib-common; - test coverage for that scenario, since `dedup-libs.py` currently has none; -- 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]
