Page MenuHomePhabricator

[events] Convert DagsterEvent from namedtuple to class
AbandonedPublic

Authored by sashank on Jul 19 2019, 9:16 PM.

Details

Reviewers
natekupp
max
Group Reviewers
Restricted Project
Summary

Converting DagsterEvent to a class so that we can create subclasses for each event type

Test Plan

unit

Diff Detail

Repository
R1 dagster
Branch
refactor-events1
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

sashank updated this revision to Diff 3063.Jul 19 2019, 9:16 PM
sashank created this revision.

Remove pdb break

sashank planned changes to this revision.Jul 19 2019, 9:51 PM
natekupp accepted this revision as: natekupp.Jul 19 2019, 9:52 PM
natekupp added a subscriber: natekupp.

looks reasonable to me!

sashank updated this revision to Diff 3065.Jul 19 2019, 10:38 PM

Use JSONEncoder

This revision is now accepted and ready to land.Jul 19 2019, 10:38 PM
This revision now requires review to proceed.Jul 19 2019, 10:52 PM
max added inline comments.Jul 19 2019, 10:58 PM
python_modules/Makefile
3 ↗(On Diff #3065)

doesn't this require pytest-xdist?

python_modules/dagster/dagster/seven/json.py
16

this should probably also handle the namedtuple case?

23

k.lstrip('_'): v for k, v in obj.__dict__.items()

26

just for consistency best to use super here

sashank updated this revision to Diff 3067.Jul 19 2019, 11:04 PM

feedback

sashank updated this revision to Diff 3068.Jul 19 2019, 11:06 PM

Remove extra comment

sashank updated this revision to Diff 3069.Jul 19 2019, 11:13 PM

Fix super call

sashank marked 3 inline comments as done.Jul 19 2019, 11:14 PM
alangenfeld added inline comments.
python_modules/dagster/dagster/seven/json.py
15

is there a test already that encodes this desired behavior? if not we should add one

sashank added inline comments.Jul 22 2019, 5:27 PM
python_modules/dagster/dagster/seven/json.py
15

test_multiline_logging_complex does

max accepted this revision.Jul 22 2019, 6:37 PM
This revision is now accepted and ready to land.Jul 22 2019, 6:37 PM
sashank abandoned this revision.Jul 23 2019, 7:23 PM