-
Notifications
You must be signed in to change notification settings - Fork 360
Filter available runtimes based on configuration setting #3114
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
Signed-off-by: Kevin Bates <[email protected]>
Signed-off-by: Kevin Bates <[email protected]>
…ects Signed-off-by: Kevin Bates <[email protected]>
Signed-off-by: Kevin Bates <[email protected]>
|
I'm moving this to draft since there are some documentation updates and potential other changes that are needed. |
Signed-off-by: Patrick Titzler <[email protected]>
Signed-off-by: Patrick Titzler <[email protected]>
Signed-off-by: Martha Cryan <[email protected]>
…is enabled Signed-off-by: Kevin Bates <[email protected]>
Signed-off-by: Martha Cryan <[email protected]>
Signed-off-by: Martha Cryan <[email protected]>
Signed-off-by: Martha Cryan <[email protected]>
Signed-off-by: Martha Cryan <[email protected]>
Signed-off-by: Patrick Titzler <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Fixed both |
Signed-off-by: Martha Cryan <[email protected]>
Signed-off-by: Martha Cryan <[email protected]>
Signed-off-by: Martha Cryan <[email protected]>
Done - probably want to add some behavior to say that no runtimes are enabled though |
Are you able to configure things such that no runtimes are enabled? And if so, is that a bad thing (assuming things are graceful)? I could see it possible to have zero runtimes enabled if a bad value was given in the config, but we should probably weigh the cost of validating that each runtime provided is legit since we may not know what runtimes exist. I suppose we could check AFTER loading runtimes, and ask "did we load everything that was asked", and, if not, warn about those "configured runtimes" that were not actually loaded. But, even then, I think we should tolerate no available runtimes if that's doable. |
|
Checked the following user scenarios: Open
Run file as pipeline:
Manage runtimes:
Export pipeline:
Run pipeline:
Manage component catalogs:
|
I did test this scenario by setting What it doesn't document though in any message is what |
Signed-off-by: Kevin Bates <[email protected]>
This may be addressed in my latest commit.
I wonder if instead of displaying ... we include the configured runtimes in the message? Something like this... |
I like it because it combines multiple pieces of troubleshooting information! I believe in other places we use [] instead of {} to denote 'sets'. Should we do the same here for consistency? |
| from elyra.util.path import get_expanded_path | ||
|
|
||
|
|
||
| class PipelineProcessorRegistry(SingletonConfigurable): |
Check failure
Code scanning / CodeQL
Missing call to `__init__` during object initialization
…s not available Signed-off-by: Kevin Bates <[email protected]>
Signed-off-by: Martha Cryan <[email protected]>
Unfortunately not |
lresende
left a comment
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.
LGTM, overall is working fine. One quick question, I built Elyra using make install-all and I can't see runtime-specific components like those described in https://elyra.readthedocs.io/en/stable/user_guide/pipeline-components.html#example-custom-components but this is the same behavior from main, so it should be unrelated.
Maybe re-raise this to question in one of our community channels? |
ptitzler
left a comment
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.
LGTM! Thank you! Great feature.
This comment was marked as resolved.
This comment was marked as resolved.
Signed-off-by: Martha Cryan <[email protected]>

This pull request allows operators to NOT expose runtime processors that might otherwise be available. By default, all three out-of-the-box runtime processors ('local', 'airflow', 'kfp') are exposed because their entry points are provided as part of Elyra's packaging. With this change, operators can select which runtime processors should be exposed by setting the command-line option
--PipelineProcessorRegistry.runtimes=<runtime>. If they wish to expose multiple runtimes, then additional--PipelineProcessorRegistry.runtimes=<runtime>options are required.CLI Configuration Option
For example, a command line of

jupyter elyra --PipelineProcessorRegistry.runtimes=airflowwill produce the following set of Elyra tiles:and the following log entries:
Similarly, a command line of

jupyter elyra --PipelineProcessorRegistry.runtimes=kfp --PipelineProcessorRegistry.runtimes=localwill produce tiles:File-based Configuration Option
Note that these options can also be set in the
jupyter_elyra_config.pyconfiguration file:such that a command of

jupyter elyrayields:What changes were proposed in this pull request?
kf-notebookDockerfileto only enable Kubeflow Pipelines runtimeHow was this pull request tested?
Implementation
ELYRA_PROCESSOR_RUNTIMESand checks various combinations, as well as manual testing to produce the screenshots, etc.Documentation
make docskf-notebookcontainer imagemake kf-notebook-image)docker run -it -p 8888:8888 elyra/kf-notebook:dev)$(jupyter --config-dir)/jupyter_elyra_config.py(should bec.PipelineProcessorRegistry.runtimes = ['kfp'])Developer's Certificate of Origin 1.1