-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Test [Backport 7.74.x] Fix for Spark driver init readiness pending #22307
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
base: 7.74.x
Are you sure you want to change the base?
Conversation
* Fix for Spark driver init readiness pending * Add changelog * Lint * Add config param for retries * Simplify description * Address comments * Update spark/assets/configuration/spec.yaml Co-authored-by: Steven Yuen <[email protected]> * ddev sync models and config --------- Co-authored-by: Steven Yuen <[email protected]> (cherry picked from commit 7477060)
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: caf90d5b2d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| except JSONDecodeError as e: | ||
| response_text = response.text.strip() | ||
| if response_text and 'spark is starting up' in response_text.lower(): | ||
| # Handle startup message based on retry configuration | ||
| if self._startup_wait_retries > 0: | ||
| self._startup_retry_count += 1 | ||
| if self._startup_retry_count <= self._startup_wait_retries: | ||
| self.log.debug( | ||
| "Spark driver not ready yet at %s (attempt %d/%d): %s", | ||
| self._get_url_base(address), | ||
| self._startup_retry_count, | ||
| self._startup_wait_retries, | ||
| response_text, | ||
| ) | ||
| return None |
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.
Avoid returning None from JSON parser
When the driver returns the startup banner, _rest_request_to_json now returns None. Callers like _driver_init immediately iterate the result (for app_json in metrics_json) without a None guard, which will raise a TypeError and still fail the check in the exact “driver starting up” scenario this change targets. This means the JSONDecodeError is replaced by an unhandled exception and the check won’t gracefully retry. Consider returning an empty list/dict or handling None in the driver-init path before iterating.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is Additional details and impacted files🚀 New features to boost your workflow:
|
Ignore this PR. Just used to test a workflow.
Backport 7477060 from #22252.
What does this PR do?
Prevents JSON decode errors when the Spark driver is starting but not quite ready.
Motivation
Customer inquiry via support ticket.
Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged