Skip to content

Commit 1327f64

Browse files
committed
Reconstruct tempfile logic in own class
1 parent 928143b commit 1327f64

File tree

3 files changed

+191
-15
lines changed

3 files changed

+191
-15
lines changed

choreographer/__init__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33
from .browser import get_browser_path
44
from .cli_utils import get_browser
55
from .cli_utils import get_browser_sync
6+
from .tempfile import TempDirectory
7+
from .tempfile import TempDirWarning
68

79
__all__ = [
810
Browser,
911
get_browser,
1012
get_browser_sync,
1113
browser_which,
1214
get_browser_path,
15+
TempDirectory,
16+
TempDirWarning,
1317
]

choreographer/browser.py

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@
1616
from .protocol import Protocol
1717
from .protocol import TARGET_NOT_FOUND
1818
from .session import Session
19-
from .system import which_browser
19+
from .system import browser_which
2020
from .tab import Tab
2121
from .target import Target
22+
from .tempfile import TempDirectory
23+
from .tempfile import TempDirWarning
2224

2325

2426
class UnhandledMessageWarning(UserWarning):
@@ -33,11 +35,8 @@ class BrowserClosedError(RuntimeError):
3335
pass
3436

3537

36-
with_onexc = bool(sys.version_info[:3] >= (3, 12))
37-
38-
3938
def get_browser_path():
40-
return os.environ.get("BROWSER_PATH", which_browser())
39+
return os.environ.get("BROWSER_PATH", browser_which())
4140

4241

4342
class Browser(Target):
@@ -69,14 +68,17 @@ def __init__(
6968
self._loop_hack = False # see _check_loop
7069
self.lock = None # TODO where else is this set
7170
self.tabs = OrderedDict()
71+
self.sandboxed = False # this is if our processes can't use /tmp
7272

7373
# Browser Configuration
7474
if not path:
7575
path = get_browser_path()
7676
if not path:
7777
raise BrowserFailedError(
78-
"Could not find an acceptable browser. Please set environmental variable BROWSER_PATH or pass `path=/path/to/browser` into the Browser() constructor. See documentation for downloading browser from python.",
78+
"Could not find an acceptable browser. Please call `choreo.get_browser()`, set environmental variable BROWSER_PATH or pass `path=/path/to/browser` into the Browser() constructor. The latter two work with Edge.",
7979
)
80+
if "snap" in str(path):
81+
self.sandboxed = True
8082
self._env["BROWSER_PATH"] = str(path)
8183
self.headless = headless
8284
if headless:
@@ -90,8 +92,9 @@ def __init__(
9092
self._env["SANDBOX_ENABLED"] = "true"
9193

9294
# Expert Configuration
93-
self._tmp_path = kwargs.pop("tmp_path", None)
94-
# TODO: stub, tempfile, must create
95+
tmp_path = kwargs.pop("tmp_path", None)
96+
self.tmp_dir = TempDirectory(tmp_path, sneak=self.sandboxed)
97+
9598
try:
9699
self.loop = kwargs.pop("loop", asyncio.get_running_loop())
97100
except Exception:
@@ -386,7 +389,7 @@ async def close_task():
386389
except ProcessLookupError:
387390
pass
388391
self.pipe.close()
389-
self._clean_temp()
392+
self.temp_dir.clean()
390393

391394
return asyncio.create_task(close_task())
392395
else:
@@ -395,7 +398,7 @@ async def close_task():
395398
except ProcessLookupError:
396399
pass
397400
self.pipe.close()
398-
self._clean_temp()
401+
self.temp_dir.clean()
399402

400403
async def _watchdog(self):
401404
self._watchdog_healthy = True
@@ -410,11 +413,13 @@ async def _watchdog(self):
410413
await self.close()
411414
await asyncio.sleep(1)
412415
with warnings.catch_warnings():
413-
# we'll ignore warnings here because
414-
# if the user sloppy-closes the browsers
415-
# they may leave processes up still trying to create temporary files
416-
# warnings.filterwarnings("ignore", category=TempDirWarning) #TODO
417-
self._retry_delete_manual(self._temp_dir_name, delete=True)
416+
# ignore warnings here because
417+
# watchdog killing is last resort
418+
# and can leaves stuff in weird state
419+
warnings.filterwarnings("ignore", category=TempDirWarning)
420+
self.temp_dir.clean()
421+
if self.temp_dir.exists:
422+
self.temp_dir.delete_manually()
418423

419424
def __exit__(self, type, value, traceback):
420425
self.close()

choreographer/tempfile.py

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import os
2+
import platform
3+
import shutil
4+
import stat
5+
import sys
6+
import tempfile
7+
import time
8+
import warnings
9+
from pathlib import Path
10+
from threading import Thread
11+
12+
13+
class TempDirWarning(UserWarning):
14+
pass
15+
16+
17+
# Python's built-in temporary directory functions are lacking
18+
# In short, they don't handle removal well, and there's lots of API changes over recent versions.
19+
# Here we have our own class to deal with it.
20+
class TempDirectory:
21+
def __init__(self, path=None, sneak=False):
22+
self._with_onexc = bool(sys.version_info[:3] >= (3, 12))
23+
args = {}
24+
25+
if path:
26+
args = dict(dir=path)
27+
elif sneak:
28+
args = dict(prefix=".choreographer-", dir=Path.home())
29+
30+
if platform.system() != "Windows":
31+
self.temp_dir = tempfile.TemporaryDirectory(**args)
32+
else: # is windows
33+
vinfo = sys.version_info[:3]
34+
if vinfo >= (3, 12):
35+
self.temp_dir = tempfile.TemporaryDirectory(
36+
delete=False,
37+
ignore_cleanup_errors=True,
38+
**args,
39+
)
40+
elif vinfo >= (3, 10):
41+
self.temp_dir = tempfile.TemporaryDirectory(
42+
ignore_cleanup_errors=True,
43+
**args,
44+
)
45+
else:
46+
self.temp_dir = tempfile.TemporaryDirectory(**args)
47+
48+
self.path = self.temp_dir.name
49+
self.exists = True
50+
if self.debug:
51+
print(f"TEMP DIR NAME: {self._temp_dir_name}", file=sys.stderr)
52+
53+
def delete_manually(self, check_only=False):
54+
if not os.path.exists(self.path):
55+
self.exists = False
56+
if self.debug:
57+
print(
58+
"No retry delete manual necessary, path doesn't exist",
59+
file=sys.stderr,
60+
)
61+
return 0, 0, []
62+
n_dirs = 0
63+
n_files = 0
64+
errors = []
65+
for root, dirs, files in os.walk(self.path, topdown=False):
66+
n_dirs += len(dirs)
67+
n_files += len(files)
68+
if not check_only:
69+
for f in files:
70+
fp = os.path.join(root, f)
71+
if self.debug:
72+
print(f"Removing file: {fp}", file=sys.stderr)
73+
try:
74+
os.chmod(fp, stat.S_IWUSR)
75+
os.remove(fp)
76+
if self.debug:
77+
print("Success", file=sys.stderr)
78+
except BaseException as e:
79+
errors.append((fp, e))
80+
for d in dirs:
81+
fp = os.path.join(root, d)
82+
if self.debug:
83+
print(f"Removing dir: {fp}", file=sys.stderr)
84+
try:
85+
os.chmod(fp, stat.S_IWUSR)
86+
os.rmdir(fp)
87+
if self.debug:
88+
print("Success", file=sys.stderr)
89+
except BaseException as e:
90+
errors.append((fp, e))
91+
92+
# clean up directory
93+
if not check_only:
94+
try:
95+
os.chmod(self.path, stat.S_IWUSR)
96+
os.rmdir(self.path)
97+
except BaseException as e:
98+
errors.append((self.path, e))
99+
100+
if check_only:
101+
if n_dirs or n_files:
102+
self.exists = True
103+
else:
104+
self.exists = False
105+
elif errors:
106+
warnings.warn(
107+
f"The temporary directory could not be deleted, execution will continue. errors: {errors}",
108+
TempDirWarning,
109+
)
110+
self.exists = True
111+
else:
112+
self.exists = False
113+
114+
return n_dirs, n_files, errors
115+
116+
def clean(self):
117+
try:
118+
# no faith in this python implementation, always fails with windows
119+
# very unstable recently as well, lots new arguments in tempfile package
120+
self.temp_dir.cleanup()
121+
self.exists = False
122+
return
123+
except BaseException as e:
124+
if self.debug:
125+
print(
126+
f"First tempdir deletion failed: TempDirWarning: {str(e)}",
127+
file=sys.stderr,
128+
)
129+
130+
def remove_readonly(func, path, excinfo):
131+
try:
132+
os.chmod(path, stat.S_IWUSR)
133+
func(path)
134+
except FileNotFoundError:
135+
pass
136+
137+
try:
138+
if self._with_onexc:
139+
shutil.rmtree(self.path, onexc=remove_readonly)
140+
else:
141+
shutil.rmtree(self.path, onerror=remove_readonly)
142+
self.exists = False
143+
del self.temp_dir
144+
return
145+
except FileNotFoundError:
146+
pass # it worked!
147+
except BaseException as e:
148+
if self.debug:
149+
print(
150+
f"Second tmpdir deletion failed (shutil.rmtree): {str(e)}",
151+
file=sys.stderr,
152+
)
153+
self.delete_manually(check_only=True)
154+
if not self.exists:
155+
return
156+
157+
def extra_clean():
158+
time.sleep(3)
159+
self.delete_manually()
160+
161+
t = Thread(target=extra_clean)
162+
t.run()
163+
if self.debug:
164+
print(
165+
f"Tempfile still exists?: {bool(os.path.exists(str(self.path)))}",
166+
file=sys.stderr,
167+
)

0 commit comments

Comments
 (0)