Page MenuHomeElementl

Warn if you run dagit without DAGSTER_HOME set
AbandonedPublic

Authored by dgibson on Feb 19 2021, 5:31 PM.

Details

Summary

Particularly once the daemon is in the mix, letting you move forward in dagit with an in-memory DagsterInstance is just asking for trouble. Warn until you set the DAGSTER_HOME environment variable.

Test Plan

Integration, launch dagit with and without DAGSTER_HOME set

Diff Detail

Repository
R1 dagster
Branch
dagitthrow (branched from master)
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 19 2021, 5:49 PM
Harbormaster failed remote builds in B26213: Diff 32023!
Harbormaster returned this revision to the author for changes because remote builds failed.Feb 19 2021, 6:32 PM
Harbormaster failed remote builds in B26216: Diff 32027!
Harbormaster returned this revision to the author for changes because remote builds failed.Feb 19 2021, 7:22 PM
Harbormaster failed remote builds in B26219: Diff 32032!

For a user who just wants to run dagit, it seems confusing to throw an error (or even a warning) for "dagster-daemon" (ie "dagster-daemon can't run using an in-memory instance..."). I think an alternative is to supplement the message ‘the scheduler daemon is not running’ (<- can't find this in the code, but the user sent this over slack) with 'DAGSTER_HOME is not set. Make sure that DAGSTER_HOME is set to the same location for Dagit and Daemon" (iff DAGSTER_HOME is actually not set)

oh, my mistake, that was supposed to say dagit not dagster-daemon. Agree we should not mention the daemon here.

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 19 2021, 9:09 PM
Harbormaster failed remote builds in B26227: Diff 32042!
Harbormaster failed remote builds in B26228: Diff 32043!
dgibson retitled this revision from Throw if you run dagit without DAGSTER_HOME set to Warn if you run dagit without DAGSTER_HOME set.Feb 22 2021, 4:14 PM
dgibson edited the summary of this revision. (Show Details)
catherinewu added inline comments.
python_modules/dagit/dagit/cli.py
104

when i see a warning, my reaction is "i guess i did something vaguely bad". in this case, i'm not sure that running dagit without setting DAGSTER_HOME is vaguely bad? defer to you though

nit: we use the phrase "ephemeral instance" in the rest of the DAGSTER_HOME warnings

python_modules/dagster/dagster/daemon/cli/__init__.py
31–32

ah, the daemon messages use the phrase "in-memory instance" but non-daemon messages use the phrase "ephemeral instance"

This revision is now accepted and ready to land.Feb 22 2021, 7:57 PM
python_modules/dagit/dagit/cli.py
104

@catherinewu Are you saying it's not so bad? Or actually bad and we should throw an error?

Original version of this diff threw, but alex raised the point that it adds friction to the initial trying out of dagit step

dgibson edited the summary of this revision. (Show Details)

in-memory => ephemeral

alangenfeld added inline comments.
python_modules/dagit/dagit/cli.py
101–102

dagit wont work at all if you use a truly ephemeral instance, thats basically for tests only. The storage_fallback scheme is to ensure its a temporary directory backed one is the fallback behavior

This revision now requires changes to proceed.Feb 22 2021, 9:19 PM
python_modules/dagit/dagit/cli.py
101–102

to be more specific "wont work at all" -> you wont be able to observe pipeline runs