-
Notifications
You must be signed in to change notification settings - Fork 4.5k
resubmit Fix #[35617] Windows compatibility for YAML JavaJarProvider #35792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @tchivs, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've addressed a critical cross-platform compatibility issue within Apache Beam's YAML JavaJarProvider. Previously, the available() method failed on Windows systems because it incorrectly used the Unix-specific 'which' command to locate the Java executable. My changes introduce platform-aware logic, utilizing 'where' on Windows and retaining 'which' on Unix/Linux, along with a robust 'shutil.which' fallback. This ensures that Windows users can now seamlessly leverage Apache Beam's YAML functionality without encountering FileNotFoundError.
Highlights
- Windows Compatibility: The JavaJarProvider.available() method in apache_beam/yaml/yaml_provider.py was updated to resolve a FileNotFoundError on Windows by replacing the Unix 'which' command with the Windows 'where' command for locating the Java executable.
- Platform-Specific Logic: I've implemented platform.system() to dynamically choose between 'where' (for Windows) and 'which' (for Unix/Linux) when attempting to find the Java executable, ensuring correct behavior across different operating systems.
- Robust Fallback Mechanism: A try-except block has been added to catch FileNotFoundError and OSError, incorporating shutil.which('java') as a cross-platform fallback to reliably locate Java, even if the initial platform-specific command fails.
- Improved Error Messaging: The error handling for cases where Java cannot be found has been refined to provide more descriptive messages, aiding in debugging.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
The Workflow run is cancelling this PR. It is an earlier duplicate of 1729654 run. |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
assign set of reviewers |
|
Assigning reviewers: R: @claudevdm for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
waiting on author |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
|
||
| def available(self): | ||
| # pylint: disable=subprocess-run-check | ||
| trial = subprocess.run(['which', subprocess_server.JavaHelper.get_java()], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should fix subprocess_server.JavaHelper.get_java() to support windows. This approach effectively removed JAVA_HOME environment handling that was considered there and affects all platforms, and leads to inconsistent result for available() and jar_provider call
Fix Windows compatibility for YAML JavaJarProvider
I submitted a PR before but the file is gone. Try again now
#35618
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-specificwhichcommand.Problem
The original implementation uses
subprocess.run(['which', java_executable])which works on Unix/Linux systems but fails on Windows withFileNotFoundError: [WinError 2] The system cannot find the file specifiedbecause Windows doesn't have awhichcommand.Solution
platform.system()to detect the operating systemwherecommand on Windows instead ofwhichwhichcommand behavior unchangedshutil.which()as a cross-platform fallback for edge casesFileNotFoundErrorandOSErrorChanges Made
JavaJarProvider.available()method inapache_beam/yaml/yaml_provider.pywhereon Windows,whichon Unix/Linux) for Java detectionshutil.whichfallback for cross-platform compatibilityTesting
Backward Compatibility
boolorNotAvailableWithReasonCode Quality
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
FileNotFoundErrorwhen using YAML providersAfter: YAML providers work seamlessly across all supported platforms
CHANGES.mdwith noteworthy changes.Additional Notes