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
Branch
events_mypy
Lint
Lint Passed
Unit
No Test Coverage

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