reiabreu commented on PR #9168: URL: https://github.com/apache/storm/pull/9168#issuecomment-6400001719
> Thanks for adding the upgrade notes and the warning for `topology.backpressure.enable`. > > A few remarks on the notes: > > 1. They use `###` sub-headings. With [Print PR upgrade notes in the generated release notes #9189](https://github.com/apache/storm/pull/9189) as currently written, everything after the first heading is cut off, so this note would come out empty. Bullets or bold text instead of sub-headings would avoid that. > 2. Some method names do not exist (`backpressure()`, `removeBackpressureFlag()`). The removed ones are `backpressureTopologies`, `topologyBackpressure`, `setupBackpressure`, `removeBackpressure` and `removeWorkerBackpressure`. > 3. Nimbus still deletes dead-topology children under the backpressure subtree when ACL check/fixup is enabled (`AclEnforcement`), so "no longer cleaned up" is only true without it. Also, `rmr` is gone since ZK 3.5, `deleteall` is enough, and the path is `<storm.zookeeper.root>/backpressure`. > > Nit: there is a double blank line in `IStormClusterState`. @rzo1 points 1 and 2 have been adressed. Point 3 has been raised as a comment. Thank you The outstanding issue is whether you agree with introducing this breaking change in a minor release. There is a precedent. We have merged an API change for 3.2.0 recently. We we now have a mechanism to include the upgrade notes in the release notes. Let me know your thoughts. -- 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]
