Page MenuHomePhabricator

Fix intermediate event messages
ClosedPublic

Authored by max on Jul 29 2019, 6:06 PM.

Details

Reviewers
schrockn
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:5bf4322b7f77: Fix intermediate event messages
Summary
Test Plan

Manual

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

max created this revision.Jul 29 2019, 6:06 PM
max edited the test plan for this revision. (Show Details)Jul 29 2019, 6:06 PM
alangenfeld added inline comments.
python_modules/dagster/dagster/core/events/__init__.py
446–449

whats wrong with the enum entries? also this should probably be ===

451–463

should we double quotes around the io names?

alangenfeld added inline comments.Jul 29 2019, 6:17 PM
python_modules/dagster/dagster/core/events/__init__.py
446–449

also this should probably be ===

facepalm

alangenfeld added inline comments.Jul 29 2019, 6:22 PM
python_modules/dagster/dagster/core/events/__init__.py
446–449

would

if object_store_operation_result.op == ObjectStoreOperationType.SET_OBJECT.value:

or

if ObjectStoreOperationType(object_store_operation_result.op) == ObjectStoreOperationType.SET_OBJECT:

be better?

alangenfeld requested changes to this revision.Jul 29 2019, 6:23 PM

prefer keeping enum reference in place since raw string could get easily missed in refactor

This revision now requires changes to proceed.Jul 29 2019, 6:23 PM
max added inline comments.Jul 29 2019, 6:27 PM
python_modules/dagster/dagster/core/events/__init__.py
446–449

the first one won't throw on a bad value

max updated this revision to Diff 3285.Jul 29 2019, 6:31 PM

Check against enum value

This revision is now accepted and ready to land.Jul 29 2019, 6:31 PM