Page MenuHomeElementl

[mypy] Events mypy and namedtuple conversion
ClosedPublic

Authored by cdecarolis on Apr 15 2021, 7:41 PM.

Details

Summary

Add type annotations to DagsterEvents, and convert namedtuples to the typed namedtuple syntax.

Test Plan

Mypy, lint, tests

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 15 2021, 8:16 PM
Harbormaster failed remote builds in B28942: Diff 35521!

switch the NamedTuples to still have __new__ but be typed

python_modules/dagster/dagster/core/events/__init__.py
897

should be specific for this func - EngineEventData

This revision now requires changes to proceed.Apr 15 2021, 9:02 PM

Change named tuple syntax, fix mypy errors

update message seems out of sync with diff - all the * need to be the NamedTuple that still has __new__ since they are loading from storage the values should be checked

python_modules/dagster/dagster/core/events/__init__.py
1130–1242

*

1281–1394

*

1413–1418

this ones fine

This revision now requires changes to proceed.Apr 19 2021, 10:03 PM
cdecarolis retitled this revision from [mypy] Events mypy to [mypy] Events mypy and namedtuple conversion.Apr 20 2021, 2:05 AM
cdecarolis edited the summary of this revision. (Show Details)

Add checks back to serdes namedtuples

sort of subtle but the * inline comments are multiline so if you hover on then you see what they apply to

all whitelist_for_serdes namedtuples need to keep their check calls

back to your queue

This revision now requires changes to proceed.Apr 21 2021, 4:17 PM

mew

python_modules/dagster/dagster/core/events/__init__.py
1136

\/

1181

\/

1228

same as comment below

1348

this should actually be List since the constructor below uses opt_list_param which coerces in to []

This revision is now accepted and ready to land.Apr 22 2021, 3:06 PM