Fix nine failing tests on main (regressor forecasting on pandas 2.2+, swallowed CLI messages, task-run 500, task-run timestamps)#2303
Merged
Conversation
- Forecasting with past/future regressors raised a TypeError on pandas 2.2+:
_slice_closed used the deprecated Series.view("int64") detour and then
set values into an existing column with .loc[:, col], which validates
against that column's dtype. Search the datetime64 values directly, and
replace the column instead of setting into it.
- CLI validation messages were swallowed for every marshmallow-typed option:
since the click 8.2 upgrade (#2219) removed the click.ParamType base,
click wraps these fields in FuncParamType, whose convert() reports the
offending *value* and discards the reason. Raise a click error carrying the
validation message instead (the ParamType base stays off, to keep the
Python 3.10 MRO fix intact).
- The task detail page returned a 500 for jobs whose meta data is not
JSON-serializable, because rq-dashboard serializes meta with a bare
json.dumps(). Give it a tolerant default (see rq-dashboard#510).
Fixes #2302
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rbix8k1JfeUWNXEmHEZVpX
Flix6x
added a commit
that referenced
this pull request
Jul 12, 2026
…essage format PR #2303 makes click report the validation message rather than the offending value, which changes the exact wording of this error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rbix8k1JfeUWNXEmHEZVpX
Documentation build overview
14 files changed ·
|
- [GET] /api/ops/getLatestTaskRun turned any database error into an opaque 500: the retry in get_task_run() ran on the still-aborted transaction, so it was rejected with InFailedSqlTransaction and the original error was never surfaced. Roll back before retrying. - test_api_task_run_get_recent_entry did not declare the fixture seeding its data, so it queried a database without the latest_task_run table at all (which is what aborted the transaction above). - test_build_asset_jobs_data and test_get_job_status_requires_read_access asserted on job counts and job identity without flushing the Redis job cache, so leftover jobs from earlier tests (which @job_cache also reuses for identical requests) made them fail depending on test order. Both now use the existing clean_redis fixture. Fixes #2302 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rbix8k1JfeUWNXEmHEZVpX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the nine tests that fail on a clean
main(see #2302). None was a stale assertion — they were masking four distinct defects, one of which breaks a production feature._slice_closed()uses deprecatedSeries.view("int64")and.loc[:, col] = <ndarray>MarshmallowClickMixin.__call__raisesValueErrorget_task_run()retries without rolling backLatestTaskRun.datetimedefault evaluated at import time1. Forecasting with regressors is broken on pandas ≥ 2.2 (4 tests)
BasePipeline.split_data_all_beliefs()raisedTypeError: value should be a 'Timestamp', 'NaT', or array of those. Got 'ndarray' instead.This is not test-only — the regressor split path itself fails, so past/future-regressor forecasting is broken on the pandas version we allow.Two pandas-version problems in four lines of
_slice_closed():Series.view("int64")— deprecated in pandas 2.2, removed in 3.0. The int64 detour is unnecessary:np.searchsortedworks ondatetime64directly.out.loc[:, "event_start"] = <ndarray>—.loc[:, col] =sets values into the existing column's dtype rather than replacing the column.2. Every CLI validation message was swallowed (1 test)
--account 9999reported the offending value but never the reason — across every marshmallow-typed option (--asset,--sensor,--source, …), not just--account.Cause: the click 8.2 upgrade (#2219) removed the
click.ParamTypebase fromMarshmallowClickMixin(fixing a Python 3.10 MRO conflict), so click wraps these fields inFuncParamType, whoseconvert()doesself.fail(value, ...)— reporting the value and discarding the message.MarshmallowClickMixin.convert()became dead code.Fix: raise
click.BadParametercarrying the validation message. TheParamTypebase stays off, so #2219's MRO fix remains intact.The option hint is preserved: click's
augment_usage_errors()attaches the offending parameter to anyBadParameterraised while that parameter is processed. So we gain the reason without losing the hint.3.
GET /api/ops/getLatestTaskRunturns any DB error into an opaque 500 (1 test)get_task_run()retries its query on aDatabaseError— but does not roll back first. A failed statement leaves the transaction aborted, so the retry is rejected withInFailedSqlTransaction, masking the original error. As written, the retry could never succeed. Now it rolls back before retrying.(The accompanying test also never declared the fixture that seeds its data, so it queried a database where
latest_task_rundid not exist — which is what aborted the transaction in the first place.)4.
LatestTaskRunwas stamped with the process start time (1 test)The default is evaluated once, at import time, so every task run created without an explicit timestamp got the time the process started — the wrong answer for a table whose only purpose is "when did this task last run?". Both production call sites happen to overwrite
.datetimeimmediately, so the impact was latent, but the default was still wrong. Now a callable.This is also what made
test_api_task_run_get_recent_entryorder-dependent: in a long test session, "import time" drifts past the test's 2-minute freshness window.Plus: two order-dependent tests (2 tests)
test_build_asset_jobs_dataandtest_get_job_status_requires_read_accessasserted on job counts and job identity without flushing the Redis job cache — and@job_cachesilently reuses a cached job for an identical scheduling request. Both now use the existingclean_redisfixture. (This is why identical full-suite runs previously produced different failure counts.)Testing
Baseline on clean
mainfor the same scope: 9 failed. Verified like-for-like (same selection, same machine) that this branch introduces no regressions.Fixes #2302
🤖 Generated with Claude Code
https://claude.ai/code/session_01Rbix8k1JfeUWNXEmHEZVpX