-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Add an experiment to make prism server a singleton #34623
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
4ef3c57
to
94c9cb0
Compare
94c9cb0
to
4122723
Compare
4122723
to
3975e89
Compare
c670b1b
to
892356d
Compare
7c48491
to
0956a9d
Compare
Assigning reviewers. If you would like to opt out of this review, comment R: @claudevdm for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
debug_options = options.view_as(pipeline_options.DebugOptions) | ||
get_job_server = lambda: job_server.StopOnExitJobServer( | ||
PrismJobServer(options)) | ||
if debug_options.lookup_experiment("enable_prism_server_singleton"): |
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.
when using this with Interactive Runner, does this need to be always a singleton?
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.
+1 can we detect or pass whether interactive runner is being used and if so use a singleton instead of requiring a flag?
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 decided to make it an experiment for the coming release 2.65.0 (#33623 (comment)), so we don't introduce breaking change 2 weeks before branch cut and have more time to bake in.
After we make prism runner as the default local runner, we can probably choose to make singleton the default as well, but this will be upon further discussion.
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.
Got it.
R: @tvalentyn for final approval |
Follow-up to #34616, addressing #33623.
Introduces an experimental feature for Prism Runner to maintain only a single background Prism server instance. This is beneficial in environments like Google Colab where long-lived Python processes may trigger Prism Runner multiple times across different pipeline executions.