aglinxinyuan opened a new pull request, #6928:
URL: https://github.com/apache/texera/pull/6928

   ### What changes were proposed in this PR?
   
   `fs` (PyFilesystem2) is effectively EOL — 2.4.16 (2022) is the last release 
ever published, upstream still imports `pkg_resources` at import time, and 
setuptools 82 removed `pkg_resources` entirely, which is the sole reason amber 
pins `setuptools==80.10.2` (#4199, #6412). `ExecutorManager` was the only 
consumer, and fs's `TempFS` is internally just `tempfile.mkdtemp` + 
`shutil.rmtree`, so the swap to stdlib is behavior-preserving.
   
   ```
   Before:  ExecutorManager -> fs.open_fs("temp://") -> pkg_resources -> 
setuptools held < 82
   After:   ExecutorManager -> tempfile.mkdtemp + shutil.rmtree -> fs gone -> 
setuptools unpinned
   ```
   
   | File | Change |
   | --- | --- |
   | `executor_manager.py` | `fs.open_fs("temp://")` → 
`tempfile.mkdtemp(prefix="texera-udf-")` + `shutil.rmtree`; property renamed 
`fs` → `tmp_dir`; lazy creation, `sys.path` handling, the never-materialized 
close guard, and the tolerated leak-on-force-kill semantics all preserved |
   | `test_executor_manager.py` | the two direct `manager.fs` accesses use 
`manager.tmp_dir`; new non-ASCII UDF round-trip regression test (see below) |
   | `requirements.txt` | drop `fs==2.4.16` and the `setuptools==80.10.2` pin — 
fs was the only distribution in the closure requiring setuptools (pytz's 
`pkg_resources` use is a guarded fallback) |
   | `LICENSE-binary-python` | drop `fs==2.4.16` and `appdirs==1.4.4` (an 
fs-only transitive); `six` stays (python-dateutil needs it) |
   | `texera_run_python_worker.py`, `pyproject.toml` | revert the #6880 warning 
filters — the warning's source is gone, so vs `main` these files are 
net-untouched (pyproject keeps only an EOF-newline fix) |
   
   One subtlety worth reviewer attention: fs's `open()` forced UTF-8 for text 
writes, while `pathlib`'s `open("w")` uses the locale encoding — on a cp1252 
Windows host, non-ASCII UDF code would crash at write time or fail at import 
(importlib decodes source as UTF-8 per PEP 3120). The swap therefore pins 
`encoding="utf-8", newline="\n"` explicitly, keeping byte-identical behavior 
with fs, and a regression test pins it.
   
   Stacked on #6880 (contains its commit); whichever merges first works — if 
this one lands first, #6880 can simply be closed.
   
   ### Any related issues, documentation, discussions?
   
   Closes #6917.
   
   ### How was this PR tested?
   
   - New `test_non_ascii_udf_source_round_trip` written first: it fails with 
`UnicodeEncodeError` under a cp1252 locale without the explicit 
`encoding="utf-8"` (reproduced on Windows), and passes with it — pinning 
encoding parity with fs.
   - The two `close()` branch tests were repointed at `tmp_dir` red-first 
(failing on the old attribute) before the source swap; all 23 
`test_executor_manager.py` tests pass.
   - Full pyamber unit suite from `amber/`: 750 passed, identical to the 
pre-change baseline apart from the added test (the handful of Iceberg-catalog 
failures on this Windows machine are identical on a clean tree). The suite's 
`pkg_resources` warnings disappeared at the source (5 → 1 total warnings) with 
no filter in place.
   - Behavior parity was checked against fs's own implementation 
(`fs/tempfs.py`, `fs/iotools.py`): directory creation, close/cleanup semantics 
including double-close, and the cached-property guard translate 1:1.
   - `ruff check` and `ruff format --check` pass on all touched files.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Fable 5)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to