Add support for running Spark Connect client inside the Dataproc s8s batch - #149
Add support for running Spark Connect client inside the Dataproc s8s batch#149ZhiweiLinnn wants to merge 3 commits into
Conversation
Summary of ChangesHello @ZhiweiLinnn, 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! This PR enables the Spark Connect client to correctly integrate with Dataproc serverless batch workloads. It introduces logic to detect batch environments and, when detected, ensures the client connects to an existing Spark session rather than provisioning a new interactive one. This is achieved by adding environment detection, a session availability polling mechanism, and modifying the session creation flow. Comprehensive unit and integration tests are included to validate these changes. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
There was a problem hiding this comment.
Code Review
Thank you for this contribution. The changes to support Dataproc batch workloads are a great addition. I've identified a couple of areas for improvement, one of which is critical regarding the return type of getOrCreate in batch mode. Please see my detailed comments below.
| except Exception as e: | ||
| logger.warning( | ||
| f"Error while polling for Spark Connect endpoint: {e}" | ||
| ) | ||
| time.sleep(5) |
There was a problem hiding this comment.
Catching a generic Exception is too broad and can mask unexpected issues or even catch system-exiting exceptions like KeyboardInterrupt. It's better to catch more specific exceptions that you expect from the get_session API call and want to retry on.
Given that this loop is waiting for a session to become available, it would be safer to only catch exceptions that indicate a transient state, such as NotFound (if the session is not yet created) or other retryable API errors. The google-api-core library provides specific exception types for this. Using the already imported exceptions would make this more robust.
| except Exception as e: | |
| logger.warning( | |
| f"Error while polling for Spark Connect endpoint: {e}" | |
| ) | |
| time.sleep(5) | |
| except (NotFound, Aborted, FailedPrecondition) as e: | |
| logger.warning( | |
| f"API error while polling for Spark Connect endpoint, will retry: {e}" | |
| ) | |
| time.sleep(5) |
For batch, we want initiated sessions.
Add integration test for batch environ