-
Notifications
You must be signed in to change notification settings - Fork 344
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
Designate kfp python package as optional dependency #3144
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Alan Chin <[email protected]>
Signed-off-by: Alan Chin <[email protected]>
Signed-off-by: Alan Chin <[email protected]>
"kfp>=1.7.0,<2.0,!=1.7.2", # We cap the SDK to <2.0 due to possible breaking changes | ||
"typing-extensions>=3.10,<5", # Cap from kfp | ||
] | ||
|
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.
might remove the newline here, small nitpicking by me
Testing this, the changes from the PR, out. Currently, still getting, due to kfp not present (no module named kfp). I did make package-ui, make build-dependencies and make build-server and then installed the wheel file with the option [gitlab] instead of [all] or [kfp-tekton] as I am only using airflow with Elyra. It is working in the sense that I now no longer have kfp in pip list ... https://elyra.readthedocs.io/en/stable/getting_started/installation.html#packaging So now I have a python environment with no kfp python package anymore, so far, so good. Except:
https://github.com/elyra-ai/elyra/blob/main/elyra/metadata/schema.py#L260 There would probably need to be a way to remove the hard-coded kfp runtime from RuntimeProcessorType enum https://github.com/kevin-bates/elyra/blob/main/elyra/pipeline/runtime_type.py#L25 Cause there is a difference between active runtimes from the config we all know and installed runtimes that then make a problem once kfp is not installed anymore. I get my hunch from @kevin-bates https://github.com/elyra-ai/elyra/pull/2263/files#diff-2cb56468c742ac4f0ea118665f54e0424289b6cf4fd47233342eebd08034a889 @harshad16 what do you think? |
brief update: I managed to separate this out correctly, making build possible without kfp dependency. Will share a PR replacing this one shortly. |
Closes #3139
What changes were proposed in this pull request?
How was this pull request tested?
Developer's Certificate of Origin 1.1