milenkovicm commented on PR #2637: URL: https://github.com/apache/datafusion-ballista/pull/2637#issuecomment-6564167083
> Thanks for the quick turnaround! Keeping `cluster_config` with the deprecation warning looks great, and the parametrized setter test is exactly what I had in mind. i would suggest not to add warning just now and see how it goes. > The `ballista.rs` change came from the tonic-prost-build bump on main, so I opened #2732 for it. Once that merges it'll drop out of this diff on its own, no need to touch it here. The benchmark formatting is fine to keep. > > One optional follow-up: now that nothing passes `config_overrides` to `with_ballista_query_planner` any more, I think that parameter and `apply_datafusion_override` in `python/src/query_planner.rs` can go. removed > Could you fill in the empty sections of the PR description? The note about #2619 can go now that it's merged. Everything else I left is non-blocking. fixed thanks @andygrove for review, it should be fixed now -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
