Technical bug: Incorrect type annotations in some superset.typing types

[giftig]

The type field has the incorrect type annotation (ironically) for at least ResultSetColumnType and SQLAColumnType. I'm not sure why mypy hasn't identified this as a problem.

This has the effect of confusing developers as to which types should be expected here, and has caused some type confusion / type coercion to spread around the db specs in some cases as we end up with an unexpected mix of strings and SQLA types.

How to reproduce the bug

• Check the actual types observed in a db_engine_spec class when handling either SQLAColumnType or ResultSetColumnType types, and compare to the annotation for the type field on these classes in both cases.

• The types are annotated as Optional[str] but are actually an SQLA type, not strings

• I suspect they're also not really optional and we're expecting them to always be present, but that's a little harder to confirm

Expected results

see strings

Actual results

see SQLA types

Environment

(please complete the following information):

• browser type and version:
• superset version: superset version
• python version: python --version
• node.js version: node -v
• any feature flags active:

Checklist

Make sure to follow these steps before submitting your issue - thank you!

• [x] I have checked the superset logs for python stacktraces and included it here as text if there are any.
• [x] I have reproduced the issue with at least the latest released version of superset.
• [x] I have checked the issue tracker for the same issue and I haven't found one similar.

Additional context

Note I'm not aware of any bugs impacting the application as a result of this; its impact is primarily technical and resulting in confusion and/or technical mess.

[giftig]

If I get a moment I'll raise a PR to change the type annotations but it might be worth investing some extra effort into figuring out why mypy didn't care that we haven't been conforming to the stated types, since it somewhat defeats the purpose of maintaining them if it doesn't.

[rusackas]

cc @john-bodley @betodealmeida @villebro for situational awareness.

[john-bodley]

Thanks for flagging this @giftig. Whilst working on https://github.com/apache/superset/pull/25844 today I first changed the only the function signature and then ran tox -e pre-commit and was surprised to see that Mypy passed even though there were functions which were calling the BaseDAO.delete()with a single item (as opposed to a list). There's definitely something a miss.

[rusackas]

Is anyone interested in working on this, or at least able to confirm it's an issue in 4.0 ? I assume it is, but the issue is at risk of being closed as stale if it's not heading anywhere.