abderbejaoui commented on PR #38490:
URL: https://github.com/apache/superset/pull/38490#issuecomment-4896866595
Thanks for the thorough pass, @rusackas! I've gone through every open thread
— applied the real ones and refuted the nits, all resolved:
**Applied:**
- Your two inline suggestions — empty input now clears
`schema_cache_timeout`/`table_cache_timeout` to unset and `cache_timeout` to
the global default (19a8c261f6), plus new `dbReducer` tests pinning the
clear-to-empty behavior.
- CodeAnt **Critical** (null API/import mismatch) — the API now rejects
`metadata_cache_timeout: null`, top-level and nested, matching
`ImportV1DatabaseExtraSchema` (19b675f5e1).
- CodeAnt **Major** (booleans passing as ints) — the validator now rejects
`true`/`false`, matching marshmallow's `fields.Integer` (1221c121e7).
**Refuted + resolved:** the Minor `[custom_rule]` type-annotation
suggestions on the test locals — the surrounding tests in that file don't
annotate `schema`/`result`/`payload`/`extra` and rely on mypy inference, so
annotating only the new ones would be inconsistent without adding type safety.
One pre-existing, out-of-scope note: marshmallow coerces `"600"`/`1.5` on
import while the API uses strict `isinstance` — happy to open a follow-up to
fully unify the two schemas if you'd like.
Heads up on CI: the new pushes re-gate the workflows ("awaiting approval"),
so they'll need a re-approval to run. All checks were green on the prior run
and these changes are lint-clean and covered by tests.
I'm around for any further review — thanks again! 🙏
--
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]
