GGraziadei commented on PR #8783: URL: https://github.com/apache/storm/pull/8783#issuecomment-4638762310
Thank you for the reviews! I have just pushed a new round of commits to address your feedback. @rzo1 Redundant Loop Execution: I refactored the logic using the new functional switch pattern to ensure the component declarer is defined only once during the first stream, eliminating the redundant calls. Validation asymmetry: Updated the logic to validate the topology-wide configuration as well, ensuring a consistent fail-fast behaviour across both levels. Documentation and nit: fixed. @reiabreu Regarding the topology-level override test: I did some further investigation. Because the merged configuration is evaluated at runtime, a full verification would technically require an integration test. However, since FluxBuilder acts as a wrapper for TopologyBuilder, and TopologyBuilder's overriding behaviour is already covered under `TopologyIntegrationTest#testComponentSpecificConfig`, I opted for a lighter approach to avoid test duplication. I have added a unit test in TCKTest.java to verify that merging the topology and component configurations via Utils.merge() executes without conflicts. If you still feel a full integration test is necessary here, please let me know, and I'll be happy to add it! -- 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]
