[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2603 @HeartSaVioR , I think changes in #2433 do take care of caching assignments within `InMemoryAssignmentBackend`, so the changes proposed in #2603 are no longer needed. I am closing this PR ---
[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2603 @kishorvpatil Now #2433 is merged in. Could you rebase and check this patch is still valid? ---
[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2603 @HeartSaVioR Sure. Let me review #2433. In the meantime, I will create patch for 1.x-branch. ---
[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2603 @kishorvpatil Could we hold on this for #2433 since #2433 is making huge change on relevant part? It would be really great if you have time to take a look at #2433 and check the conflict between this and #2433. Btw, regardless of #2433 I think this change can go through 1.x-branch, though separate patch is needed. ---
[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2603 @d2r Thank you for the review. I have addressed the comments. The failure seems unrelated to my changes - the test passes on local environment. ---
[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus
Github user d2r commented on the issue: https://github.com/apache/storm/pull/2603 The CI test error was in the nimbus test, `test-check-authorization-getSupervisorPageInfo`, and seems unrelated. @kishorvpatil does it look that way to you? ---
