Repository navigation
Task SDK: Skip POSIX-only supervisor and task runner tests on Windows - #70946
Adityakk9031 wants to merge 1 commit into
Conversation
|
@potiuk have a look |
|
Please split those even further - each for different area. Some of them some prek, some tests, some production. |
uros-b
left a comment
There was a problem hiding this comment.
@Adityakk9031 Please fix the PR title, otherwise code changes look good!
|
This pull request has been automatically marked as stale because the author has not responded to a request for more information. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
|
@uros-b changed please merge it |
|
Thank you @Adityakk9031, please ping @potiuk here for further review |
b291c67 to
2de5d15
Compare
|
@potiuk @uros-b Per feedback, I have split the changes into focused micro-PRs by area:
|
potiuk
left a comment
There was a problem hiding this comment.
Thanks for splitting this up. The scope is right now: only the test skips, with the prek and shared/stats parts in their own PRs. The skips themselves need to be narrower:
test_task_runner.py: the module-wide skip hides about 187 tests, buttask_runner.pyuses no POSIX-only process primitives that I can find. It only installs aSIGTERMhandler, which exists on Windows, andtimeout.pyalready falls back whenSIGALRMis missing. Could you share which tests actually fail on Windows (a pytest summary with the errors), and mark only those, with a reason that names the API involved?test_supervisor.pyalready uses precise per-test markers (around lines 5145–5246, and thewin32skip at 5412). Please follow that pattern: mark the classes or tests that spawn processes (os.fork/posix_spawn/setpgid/killpg) rather than the whole module, so tests that don't need POSIX keep running.- The description says "Close: #69464", but that issue is the Sentry
before_sendbug and unrelated to this change. Please remove or fix the reference so merging doesn't close it.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
2de5d15 to
2406480
Compare
|
@Adityakk9031 I don't think we say anywhere that Airflow support Windows! If your goal is to ensure it runs with Windows, please upload a demo window or screenshot of it working end to end |
Skip POSIX-only supervisor and task runner tests on Windows using
pytestmarkrather than stubbing out POSIX syscalls. CI on Linux runs all tests as normal.Per maintainer request (@potiuk), the Windows compatibility changes have been split into separate, focused PRs:
common_prek_utils.py: Ensure explicit UTF-8 encoding in prek scripts for Windows compatibility #73679register_at_forkguard: Observability: Guard register_at_fork in shared stats for Windows compatibility #73680Related to #69464 (not closing it).