FrancescoCastaldi commented on PR #45822: URL: https://github.com/apache/superset/pull/45822#issuecomment-6509106331
### Executive Summary & Review Feedback Resolution All feedback from @sadpandajoe on PR #45822 has been addressed, verified, and pushed to origin in commit `e74e08b55c`. #### 1. Addressed Review Items & Foresight Anticipation - **`QueryObject` Filter Parameter Fix**: corrected `tests/unit_tests/models/helpers_test.py` to use `filters=` instead of `filter=`, preventing clauses from being swallowed by `**kwargs`. - **Relative Offset Filter-Rewrite Verification**: structured `test_processing_time_offsets_malformed_relative_filter_raises_validation_error` with a valid outer time range and non-datetime index series alongside a malformed second temporal filter (`col="other_ds", val="a : b"`) and relative offset (`"1 day ago"`). Verified that `QueryObjectValidationError` names `other_ds` and chains `TimeRangeParseFailError` on `__cause__`. - **Date Range Timeshifts Path (`DATE_RANGE_TIMESHIFTS_ENABLED`)**: added `test_processing_time_offsets_malformed_date_range_offset_raises_validation_error` covering inverted date ranges like `"2024-02-01 : 2024-01-01"` when date range timeshifts are enabled, verifying the exception chaining. - **Outer Time Range Validation**: added `test_processing_time_offsets_malformed_outer_time_range_raises_validation_error` for outer temporal range parsing failures. - **`get_sqla_query` Chaining**: added assertions confirming `TimeRangeParseFailError` is chained on `__cause__`. - **Elimination of Translation Template Drift**: in `superset/models/helpers.py`, sanitized `QueryObjectValidationError` message strings without `_()`, resolving the pot template drift detected by `scripts/translations/check_pot_drift.py` and `babel-extract`. #### 2. Modified Files & Test Coverage - `superset/models/helpers.py`: updated validation exception strings and exception chaining. - `tests/unit_tests/models/helpers_test.py`: added 3 comprehensive unit test suites covering outer range, relative offset filter-rewrite, and date range timeshifts. #### 3. Evidence Gate & Quality Checks - Line length audit: 100% compliant with Ruff and Black strict 88-column limit. - Python syntax compilation: passed with zero errors. - Translation check (`babel-extract`): "Check messages.pot matches a fresh extraction" passed with success. @sadpandajoe, all requested test improvements and CI fixes are in place. The PR is ready for your re-review whenever you have a moment, thank you! -- 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]
