Skip to content

Conversation

@tchivs
Copy link
Contributor

@tchivs tchivs commented Jul 17, 2025

Fix Windows compatibility for YAML JavaJarProvider

Description

This PR fixes a cross-platform compatibility issue in the YAML provider where the JavaJarProvider.available() method fails on Windows systems due to the use of the Unix-specific which command.

Problem

The original implementation uses subprocess.run(['which', java_executable]) which works on Unix/Linux systems but fails on Windows with FileNotFoundError: [WinError 2] The system cannot find the file specified because Windows doesn't have a which command.

Solution

  • Platform detection: Use platform.system() to detect the operating system
  • Windows compatibility: Use where command on Windows instead of which
  • Unix/Linux compatibility: Keep existing which command behavior unchanged
  • Fallback mechanism: Add shutil.which() as a cross-platform fallback for edge cases
  • Error handling: Improved error handling for FileNotFoundError and OSError

Changes Made

  • Modified JavaJarProvider.available() method in apache_beam/yaml/yaml_provider.py
  • Use platform-specific commands (where on Windows, which on Unix/Linux) for Java detection
  • Add shutil.which fallback for cross-platform compatibility
  • Maintain backward compatibility with existing Unix/Linux behavior

Testing

  • ✅ Tested on Windows 10/11
  • ✅ Tested on Linux Ubuntu

Backward Compatibility

  • Fully backward compatible - no breaking changes
  • Unix/Linux behavior unchanged - existing functionality preserved
  • Same return types - maintains bool or NotAvailableWithReason
  • Same error messages - consistent user experience

Code Quality

  • ✅ Follows existing code style and conventions
  • ✅ Includes proper error handling for all edge cases
  • ✅ Maintains the same method signature and return types
  • ✅ Added appropriate inline comments
  • ✅ Preserves existing pylint disable comments

Impact

This change enables Windows users to use Apache Beam's YAML functionality without requiring workarounds, manual patches, or Unix-like environments (WSL, Git Bash, etc.).

Before: Windows users encounter FileNotFoundError when using YAML providers
After: YAML providers work seamlessly across all supported platforms


  • fixes #35617
  • Update CHANGES.md with noteworthy changes.

Additional Notes

  • This is a small, focused fix that addresses a specific cross-platform compatibility issue
  • No ICLA required as this is a minor bugfix
  • Ready for review and testing on multiple platforms

@github-actions
Copy link
Contributor

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

@damccorm damccorm merged commit db0c17f into apache:master Jul 17, 2025
1876 of 1890 checks passed
@tchivs
Copy link
Contributor Author

tchivs commented Jul 18, 2025

why the number of changed files shows as 0 and my committed code seems to have disappeared.

@damccorm
Copy link
Contributor

damccorm commented Jul 18, 2025

I'm not sure - this is quite confusing. It also says I merged your PR, but I did not see this PR until now...

I did merge #35316 at the same time. Seems like maybe a github bug? This repo's main branch doesn't seem to have any changes similar to what I assume this PR added

@jonuchauhan
Copy link

Hi, I guess there is some issue with this PR as there is no modification in apache_beam/yaml/yaml_provider.py.

@tchivs
Copy link
Contributor Author

tchivs commented Aug 6, 2025

Hi, I guess there is some issue with this PR as there is no modification in apache_beam/yaml/yaml_provider.py.

Yes, I think I need to try to resubmit the file

@tchivs
Copy link
Contributor Author

tchivs commented Aug 6, 2025

I'm not sure - this is quite confusing. It also says I merged your PR, but I did not see this PR until now...

I did merge #35316 at the same time. Seems like maybe a github bug? This repo's main branch doesn't seem to have any changes similar to what I assume this PR added
I submitted a new PR . Try again now
#35792

@Abacn
Copy link
Contributor

Abacn commented Aug 6, 2025

I'm not sure - this is quite confusing. It also says I merged your PR, but I did not see this PR until now...

I did merge #35316 at the same time. Seems like maybe a github bug? This repo's main branch doesn't seem to have any changes similar to what I assume this PR added

It appears when someone merged a PR onto master branch, it will trigger close empty open PR.

@tchivs
Copy link
Contributor Author

tchivs commented Aug 7, 2025

我不确定 - 这很令人困惑。它还说我合并了你的 PR,但我直到现在才看到这个 PR......
我确实同时合并了 #35316。看起来可能是 github 错误?这个存储库的主分支似乎没有任何类似于我假设这个 PR 添加的更改

当有人将 PR 合并到 master 分支时,它会出现,它将触发关闭空打开 PR。

I'm not sure - this is quite confusing. It also says I merged your PR, but I did not see this PR until now...
I did merge #35316 at the same time. Seems like maybe a github bug? This repo's main branch doesn't seem to have any changes similar to what I assume this PR added

It appears when someone merged a PR onto master branch, it will trigger close empty open PR.

It seems that this is the case. My master branch automatically synchronized with the main warehouse, causing the code to be lost. I think I need to create another branch to merge pr, so that there should be no problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants