ylemiesa57 opened a new pull request, #70233:
URL: https://github.com/apache/airflow/pull/70233
Picking this one up because it looked like a well contained bug and a good
way to learn how the ssh provider's remote job cleanup works.
## What I changed
`SSHRemoteJobOperator` lets you set a custom `remote_base_dir` for where job
files get created on the remote host. The job actually runs fine under that
custom directory, but when the operator tries to clean up afterward,
`_validate_job_dir()` in `remote_job.py` was always checking the job dir
against the hardcoded `POSIX_DEFAULT_BASE_DIR`/`WINDOWS_DEFAULT_BASE_DIR`
constants instead of whatever `remote_base_dir` was actually configured. So
cleanup would raise a `ValueError` ("Expected path under ...") for anyone not
using the default directory, even though nothing was actually wrong.
I gave `_validate_job_dir()`, `build_posix_cleanup_command()`, and
`build_windows_cleanup_command()` an optional `base_dir` parameter, and
`_cleanup_remote_job()` now passes `self.remote_base_dir` through so validation
checks against the directory the job actually used. When `base_dir` isn't
passed (or is `None`) it still falls back to the old default, so this shouldn't
change behavior for anyone not customizing `remote_base_dir`.
## Testing
- `pytest tests/unit/ssh/utils/test_remote_job.py
tests/unit/ssh/operators/test_ssh_remote_job.py` — 49 passed (46 existing + 3
new)
- Added 3 tests covering: a custom base_dir being accepted, a path outside a
custom base_dir still being rejected, and the Windows equivalent
- Confirmed the new tests actually catch the bug by checking them against
the old code first (they fail with a clear `TypeError`/would fail with the old
`ValueError` behavior)
- `ruff check` and `ruff format --check` clean on the changed files
- `mypy` clean on the two changed source files
Still fairly new to the codebase so let me know if I should tweak anything,
especially if there's a reason the validation was written against the fixed
default in the first place.
Fixes #69813
--
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]
