Page MenuHomeElementl

rename EventRecord -> EventLogEntry
ClosedPublic

Authored by yuhan on Jun 16 2021, 11:07 PM.

Details

Summary

depends on D8418
rename EventRecord to EventLogEntry
register_serdes_tuple_fallbacks seems to handle everything - except the cases when dagit or daemon is running older versions, which we will have to ask users to redeploy to pick up the serdes backcompt.

Test Plan

bk
dagit loads
historical runs load
pipeline failure sensors works

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

python_modules/dagster/dagster/core/events/log.py
174

I'm a little worried about this with https://dagster.phacility.com/D8423.... I know that the *new* EventRecord is not marked for serdes currently, but I could see this being a pretty big gotcha down the road.

Does the fact that the typename of the original EventRecord definition is _EventRecord help disambiguate at all?

python_modules/dagster/dagster/core/events/log.py
174

that's right.. i don't think the typename will make any difference since we don't really persist that. even it does, the gotcha will still be there and instead become "dont set the new EventRecord's typename to be _EventRecord". I didn't see a good way to avoid all caveat if we decide to reuse the same name.

I guess an alternative name for D8423 is EventLogRecord which is a bit verbose but actually does pair with "EventLogStorage" (as for "RunRecord" to "RunStorage").

prha added inline comments.
python_modules/dagster/dagster/core/events/log.py
174

👍

This revision is now accepted and ready to land.Jun 17 2021, 2:21 PM
This revision was automatically updated to reflect the committed changes.