Page MenuHomePhabricator

implement compute log tailing in python (enable windows mirror)
ClosedPublic

Authored by prha on Wed, Oct 9, 11:08 PM.

Details

Summary

In order to support Windows, we needed to do the following things:

A) redirect io streams to disk (currently broken in Windows for py3.6+)
B) implement a python-based tail process
C) clean up spawned processes upon segfaults in the parent process where computation occurs

This diff disables compute log capture for Windows py3.6+, unless the environment variable PYTHONLEGACYWINDOWSSTDIO is set. It prompts the user with a warning message if this is not set upon spinning up dagit.

It also implements the python-based tail process for windows systems, exposed in the dagster cli command so that it can be invoked using subprocess.Popen.

The exposed tail process polls the file (on windows) until the parent process terminates it or exits.

As a bonus, also changed the posix tail process invocation to pass in the appropriate out/err stream so that command line invocations can route the individual streams appropriately (e.g. dagster pipeline execute ... 2>/dev/null).

Test Plan

bk, wrote homegrown solid that called os._exit(1) that did not orphan a tail process (could not figure out how to write a good pytest for that).

Spun up dagit on windows, ran a job, saw logs in dagit, also on command line.

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

prha created this revision.Wed, Oct 9, 11:08 PM
prha updated this revision to Diff 5768.Thu, Oct 10, 10:13 PM

use psutil for windows

prha updated this revision to Diff 5784.Fri, Oct 11, 8:18 PM

update

prha edited the summary of this revision. (Show Details)Fri, Oct 11, 8:58 PM
prha edited the test plan for this revision. (Show Details)
prha added a reviewer: Restricted Project.
alangenfeld accepted this revision.Mon, Oct 14, 4:21 PM
alangenfeld added a subscriber: alangenfeld.

its kinda gross but nothing objectionable

push_n_pray

python_modules/dagster/dagster/core/execution/compute_logs.py
72–78

waw

This revision is now accepted and ready to land.Mon, Oct 14, 4:21 PM