make kfp dependency optional in build and during runs #3248
Conversation
shalberd
requested review from
akchinSTC,
harshad16,
romeokienzler and
lresende
shalberd
force-pushed
the
make_kfp_depedency_optional
branch
from
5599f50
to
50dc717
Compare
…til runtimes determined, make elyra.pipeline.kfp.kfp_authentication import optional only when kfp available in build Signed-off-by: shalberd <21118431+shalberd@users.noreply.github.com>
shalberd
force-pushed
the
make_kfp_depedency_optional
branch
from
50dc717
to
0fccf45
Compare
shalberd
added
component:build
There was a problem hiding this comment.
Hi @shalberd - this is great - thank you!
My only comment is regarding a couple messages displayed in the console output example, who's statements I had expected to see in this PR (but also am now unfamiliar with elyra's logging, so must have been there):
[I 2024-09-17 17:54:53.087 ElyraApp] Although runtime 'kfp' is installed, it is not in the set of configured runtimes ['airflow'] and will not be available.
[I 2024-09-17 17:54:53.087 ElyraApp] Although runtime 'local' is installed, it is not in the set of configured runtimes ['airflow'] and will not be available.
These statements seem a little superfluous with respect to the PR and I'm not sure they provide any value. I'm also wonder what "runtime 'local' is installed" means. Is there an option not to install the 'local' runtime?
Similarly for 'kfp', now that I think about it. Do you happen to have output in which 'kfp' is not installed to really show the separation? I might be forgetting lower level details regarding deployment/installation.
Thanks,
Kevin.
|
Hi @kevin-bates that message, that a runtime is installed, but not used, is related to this work on PipelineProcessorRegistry config https://github.com/elyra-ai/elyra/blob/main/elyra/pipeline/registry.py#L65 Basically, all runtimes (kfp, airflow, local) are always installed in terms of schemas https://github.com/elyra-ai/elyra/tree/main/elyra/metadata/schemas runtimeprocessors https://github.com/elyra-ai/elyra/blob/main/elyra/pipeline/runtime_type.py#L25 and so on.
They are just not enabled in the application config, not exposed and thus not called, e.g. I disabled all runtimes except airflow by setting
In essence, in the Jupyterlab GUI, I was only able to see the the Airflow pipeline editor, custom components for Airflow only, and so on. No kfp-related stuff, no "run locally" - only "run in Airflow", that all seems correctly disabled, i.e. the code not being accessed, via that old PR 3114 that you and Patrick worked on back then. 
I can completely confirm the experience in terms of GUI from PR 3114, with the added bonus that now, the kfp-related python packages are not installed (see my pip list from the main body of this PR). Now, for directory, filesystem, and url component catalog, I am still figuring out why those even appear when I run locally (I don't use them) 
and why the runtime kubeflow pipelines still appears there. That has nothing do do with existing code, though, I think. More with the schemas json. In Open Data Hub Workbench Images Jupyter Builds with Elyra, I was able to configure the runtimes in that dropwdown with a custom json component-runtime.json or I think we even disabled that completely (them for kfp in ODH, me for Airflow) @guimou @harshad16 what is the reason for disabling those component catalogs in custom workbench images? Is the fact that when the commands in the jupyter snippet are not executed and the custom json is put in, we see all types of component catalogs with all types a runtimes, is that a reaction to a deficiency in Elyra? i.e. you replace the two runtimes in the schema, e.g. at with the value from I never used those three types of component catalogs (url, file, directory) with Airflow, in any case. If it is a deficiency, maybe I could instead add programmatic logic to exclude runtimes in those schema configs, too, when parsing them, as configured in As of now, I think, component_parser for those component catalogs just takes verbatim whatever is in the schema json / runtime_type enum In my case, cause KUBEFLOW_PIPELINES single enum value is not in the list of historical: |
|
There should be a better programmatic way to showing the runtimes sans-Airflow or sans-KFP in the schema catalogs than replacing them like this in I see ODH Elyra does not handle this programmatically currently as well, just by replacement in the json. In any case, since the main goal here is to make kfp dependency optional in terms of installed python packages, and showing that only Airflow, but not KFP or local pipeline editor and runtime are shown when I only enable Airflow, I think the PR archieves what ticket #3139 wanted to achieve. |
|
@harshad16 for ODH KFP installs, you can either keep installing with @paulovmr let me know so we can sync up with what you did downstream on odh-elyra. I think removing code related to Airflow is a bit overkill downstream, maybe we can make the schema support more configurable. I did not have any issues with kfp code / processors running so far when working with the mods here w/o kfp and install option elyra[gitlab]. |
Signed-off-by: Luciano Resende <lresende@apple.com>
|
thank you so much, @lresende I have one small question for my personal understanding in the review files section pyproject, regarding airflow option and empty square brackets. |
|
Yes, I initially got a little confused as we had a lot of details on [kfp] but nothing really on [airflow] so I made the empty one mostly for simetry and to clarify on the docs. ... |
|
BTW, Just trying to get builds in green before merging... |
|
ok, yes, I think it is good to have that airflow option, i.e. for core dependencies, and then airflow-gitlab for core dependencies plus python-gitlab. |
closes #3139
closes #3144
@mamurak @RHRolun I'd appreciate your opinion on this, too. @akchinSTC got me thinking on this separation and getting rid of the need to always include kfp in builds. It's working nicely for me in the context of Airflow-related Elyra builds.
kfp python package as optional dependency
defer processor loading until runtimes determined
make elyra.pipeline.kfp.kfp_authentication import optional for only when kfp available in build
tested with Elyra Dev 3.16 Build wheel file pip install with only package extra gitlab i.e.
ELYRA_EXTRAS=[gitlab]jupyter_elyra_config.py with only airflow activated:
Working nicely at build and runtime for only airflow. no more kfp dependency for me that way when I only want airflow and possibly local processor.
Pip list in Jupyterlab, no more kfp and related packages:
Output on jupyterlab start:
What changes were proposed in this pull request?
How was this pull request tested?
Developer's Certificate of Origin 1.1