Use system temp directory for temporary HTML files#1296
Conversation
Fixes #1295. The to_html() methods in foliumap, leafmap, kepler, and heremap were creating temporary files in the current working directory using random_string(). This fails in read-only environments like hardened Docker containers. Changed to use temp_file_path() which writes to the system temp directory instead.
There was a problem hiding this comment.
Pull request overview
Updates to_html() implementations to generate temporary HTML output in the system temp directory (instead of the current working directory), addressing failures in read-only runtime environments (e.g., hardened Docker containers) and aligning behavior across backends.
Changes:
- Replaced
os.path.abspath(random_string() + ".html")temp-file generation withtemp_file_path(".html")in multiple backends. - Updated
heremap.pyimports to includetemp_file_pathfor direct use.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| leafmap/leafmap.py | Switches default to_html() temp outfile to common.temp_file_path(".html"). |
| leafmap/kepler.py | Switches default to_html() temp outfile to common.temp_file_path(".html"). |
| leafmap/heremap.py | Uses temp_file_path(".html") for default to_html() temp outfile and updates imports accordingly. |
| leafmap/foliumap.py | Switches default to_html() temp outfile to common.temp_file_path(".html"). |
Comments suppressed due to low confidence (1)
leafmap/foliumap.py:2524
- When
outfileis auto-generated viacommon.temp_file_path(), the file is removed only on the success path. Ifself.save()or the subsequent read fails, the temp file can be left behind in the system temp dir. Consider usingtry/finallyso the implicit temp file is always cleaned up.
outfile = common.temp_file_path(".html")
self.save(outfile, **kwargs)
out_html = ""
with open(outfile) as f:
lines = f.readlines()
out_html = "".join(lines)
os.remove(outfile)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| outfile = common.temp_file_path(".html") | ||
| save = False |
There was a problem hiding this comment.
This change is meant to fix failures in read-only working directories, but the existing tests calling Map.to_html() don’t assert that it works when CWD is read-only. Please add a unit test that changes into a read-only temp directory (skip on platforms where chmod isn’t reliable) and verifies to_html() succeeds without an outfile.
leafmap/foliumap.py
Outdated
| outfile = common.temp_file_path(".html") | ||
| self.save(outfile, **kwargs) |
There was a problem hiding this comment.
This change is meant to fix failures in read-only working directories, but the test suite doesn’t currently cover calling to_html() with no outfile while CWD is read-only. Please add a unit test that runs foliumap.Map().to_html() from a read-only directory and asserts it still returns HTML.
leafmap/heremap.py
Outdated
| from .common import gdf_to_geojson, random_string, shp_to_geojson, vector_to_geojson | ||
| from .common import ( | ||
| gdf_to_geojson, | ||
| random_string, |
There was a problem hiding this comment.
random_string is now imported from .common but no longer used after switching to temp_file_path(). Please remove the unused import to avoid flake8/pyflakes F401 unused-import failures.
| random_string, |
| else: | ||
| outfile = os.path.abspath(common.random_string() + ".html") | ||
| outfile = common.temp_file_path(".html") | ||
| save = False |
There was a problem hiding this comment.
When outfile is auto-generated via common.temp_file_path() (i.e., save=False), the temp file is only deleted on the happy path. If self.save()/read fails, the file can be left behind in the system temp dir. Consider wrapping the save/read in a try/finally to ensure the temp file is removed when it was created implicitly.
| else: | ||
| outfile = os.path.abspath(common.random_string() + ".html") | ||
| outfile = common.temp_file_path(".html") | ||
| save = False | ||
|
|
There was a problem hiding this comment.
When outfile is auto-generated via common.temp_file_path() (i.e., save=False), the temp file is only deleted on the happy path. If save_to_html() or file read fails, the file can be left behind in the system temp dir. Consider using try/finally to guarantee cleanup when outfile is implicitly created.
| else: | ||
| outfile = os.path.abspath(random_string() + ".html") | ||
| outfile = temp_file_path(".html") | ||
| save = False |
There was a problem hiding this comment.
When outfile is auto-generated via temp_file_path() (i.e., save=False), the temp file is only deleted on the happy path. If embed_minimal_html()/file read fails, the temp file can be left behind in the system temp dir. Consider wrapping file generation/read in try/finally so cleanup always happens when the outfile is implicitly created.
|
🚀 Deployed on https://69975835e25ed4007b470bea--opengeos.netlify.app |
- Remove unused `random_string` import from heremap.py - Add try/finally to ensure temp files are cleaned up on errors in foliumap, leafmap, kepler, and heremap - Add unit test verifying to_html() works in a read-only directory
1 participant
Summary
to_html()methods infoliumap,leafmap,kepler, andheremapwere creating temporary HTML files in the current working directory usingrandom_string(). This fails in read-only environments (e.g., hardened Docker containers with read-only app directories).temp_file_path(".html")which writes to the system temp directory (/tmp) instead. This matches the existing pattern already used indeckgl.py.Test plan
to_html()without an outfile argument works correctly in foliumap, leafmap, kepler, and heremapto_streamlit()works correctly (it callsto_html()internally)--read-onlyflag)