rusackas commented on PR #39751: URL: https://github.com/apache/superset/pull/39751#issuecomment-4753211191
Spent a while digging into why this kept dying in CI, and it turned out to be three separate things, so writing down what we found in case it's useful later. The real culprit was `marshmallow-sqlalchemy`. This PR had it pinned to `>=1.4.2`, but 1.4.1 introduced a memory regression and 1.4.2 didn't actually fix the root cause... only 1.5.0 does (marshmallow-code/marshmallow-sqlalchemy#665, fixed by @mistercrunch). With 1.4.x the `unit-tests` job just eats memory and gets killed around 11% in, which is why it kept looking like a hang on one test file. Totally invisible locally if your machine has enough RAM, which threw me for a good while. Bumped to `>=1.5.0` and the suite runs clean again. The other two were smaller. marshmallow 4 made `Schema._init_fields` strict, so it now raises `KeyError` on the relationship names FAB drops into `Meta.fields` without declaring them. Rewrote the compat shim to stub those as `Raw` fields in a single pass (the original retry-loop version is pathologically slow under coverage, and a per-class cache doesn't help since FAB mints a fresh schema class on every call). And one dashboard test was passing a `MagicMock` that happens to be subscriptable, so marshmallow read `dash["changed_on"]` instead of the attribute and choked on 4.x's stricter `DateTime` serializer... made the mock non-subscriptable like a real `Dashboard`. Plus the usual 4.x adoption bits (`@validates(**kwargs)`, `Number` -> `Float`, `default` -> `dump_default`). Full python unit suite is green now (8315 tests, ~12.5 min, back to `master`'s normal speed), and I verified each fix against real marshmallow 4 + marshmallow-sqlalchemy 1.5.0 locally. Regression tests added in `tests/unit_tests/test_marshmallow4_compat.py`. Should be good to review/merge once someone gives it a 👍. -- 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]
