fix: _set_keyspace_for_all_pools passes only the last pool's errors to callback#915
fix: _set_keyspace_for_all_pools passes only the last pool's errors to callback#915mykaul wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIn 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| errors[pool.host] = host_errors | ||
|
|
||
| if not remaining_callbacks: | ||
| callback(host_errors) | ||
| callback(errors) | ||
|
|
||
| for pool in tuple(self._pools.values()): | ||
| pool._set_keyspace_for_all_conns(keyspace, pool_finished_setting_keyspace) |
There was a problem hiding this comment.
Can you add a regression test for that?
There was a problem hiding this comment.
Isn't test_set_keyspace_for_all_pools_reports_all_errors the test for it?
…o callback The pool_finished_setting_keyspace closure was calling callback(host_errors) instead of callback(errors), so when the last pool to finish had no errors, the callback received an empty list instead of the accumulated error dict from all pools. This caused _set_keyspace_completed to report success (not [] is True) even when other pools had failures — keyspace changes silently succeeded with partial failures dropped. Introduced in d281b50 (2013-10-11). Fixes #914
8990563 to
93ca9e5
Compare
Fixes #914
pool_finished_setting_keyspacewas callingcallback(host_errors)(the last pool to finish) instead ofcallback(errors)(all accumulated errors).This caused keyspace changes to silently report success when the last pool finished without errors, even if other pools had failures.
Introduced in
d281b50c9(2013-10-11).