AmoghAtreya commented on PR #42609:
URL: https://github.com/apache/superset/pull/42609#issuecomment-5139738742
Thanks for the thorough review @aminghadersohi ! Really appreciate the deep
dive on my work.
Here is a summary of how all your feedback points were addressed:
1. **`type=operation` KeyError Fix:** Fixed and verified—`type=operation`
was updated to `operation=operation` in `query_object.py` so string
interpolation works as expected.
2. **`setdefault` / Overriding Client Bounds:** Replaced `setdefault` with
direct assignment (`resolved["time_range_start"] = self.from_dttm` and
`resolved["time_range_end"] = self.to_dttm`) so client-supplied post-processing
options cannot bypass server-resolved boundaries.
3. **Bucket Safety Cap (Memory Protection):** Added `MAX_RESAMPLE_BUCKETS =
10_000` in `utils.py`. Before materializing the resampled frame, `resample()`
now calculates/estimates the target bucket count across the span. If it exceeds
the limit, it immediately raises an `InvalidPostProcessingError`—protecting
workers from OOMs even if a caller passes fine-grained rules across large
ranges directly.
4. **Empty Result Sets & Documentation:** Added explicit docstrings on
`resample()` explaining the upstream limitation in `helpers.py` (which bypasses
`exec_post_processing` for 0-row DataFrames), while ensuring `resample()`
itself cleanly expands empty frames if ever invoked directly.
5. **Test Coverage:** Added comprehensive tests in `test_resample.py` and
`query_object_test.py` covering:
- Rejection of queries exceeding `MAX_RESAMPLE_BUCKETS`.
- Empty DataFrame expansion with and without explicit bounds.
- Server-resolved bounds taking precedence over client options.
All 38 unit tests are also passing as intended.
--
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]
