Page MenuHomeElementl

Don't pass a dir to our TemporaryDirectory
ClosedPublic

Authored by jordansanders on Fri, Apr 16, 5:09 PM.

Details

Summary

In response to:
https://buildkite.com/dagster/dagster-diffs/builds/17412#22a8ea10-289e-4a55-8d47-d58efcf92b8e

The step succeeded when retried. I was unable to reproduce the same
failure locally, but I'm incidentally able to produce a different
failure:

FileNotFoundError: [Errno 2] No such file or directory: 'user_in_loop/tmpzayi1d_n'

In short, I'm unable to specify a dir that doesn't yet exist. Creating
the TemporaryDirectory works just fine if I pass an existing directory.

Effectively, I'm running into:

>>> import os
>>> os.mkdir("foo/bar")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: 'foo/bar'
>>> os.mkdir("foo")
>>> os.mkdir("foo/bar")

This is because mktmp doesn't set parents=True when it calls mkdir:

https://github.com/python/cpython/blob/0ad81d4db2f409d72f469d0b74ab597be772a68e/Lib/tempfile.py#L364

So on a FileNotFoundError, it raises instead of recursively building
the parents:

https://github.com/python/cpython/blob/0ad81d4db2f409d72f469d0b74ab597be772a68e/Lib/pathlib.py#L1227-L1229

We don't run into this when running via tox because tox is changing our
working directory to /workdir/examples/user_in_loop so the parent
already exists.

This is the only place in our codebase where we're explicitly specifying
a dir when creating a TemporaryDirectory and it doesn't seem to be
providing any actual value to the test so I'm going to remove the dir
argument. That way, we can invoke this test locally without running it
through tox.

Test Plan

bk

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

examples/user_in_loop/tests/test_user_in_loop_pipeline.py
30

Incidentally, I kind of prefer the pytest tmpdir fixture for tests instead of using the standard library's tempfile because I think it can help guard against some of the common tempfile gotchas:

https://docs.pytest.org/en/stable/tmpdir.html

But I want to narrowly scope this diff to just the particular issue I was running into.

This revision is now accepted and ready to land.Fri, Apr 16, 5:36 PM