-
Notifications
You must be signed in to change notification settings - Fork 342
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
Initial changes to support runtime sub package installs #2259
base: main
Are you sure you want to change the base?
Conversation
Thanks for making a pull request to Elyra! To try out this branch on binder, follow this link: |
@akchinSTC Could you please add to the description, the supported syntaxes you have in mind. |
@lresende
|
How would the # 2 work? Basically don't continue processing the entrypoint load if a given dependency is not there? I think that would be a simple way to implement this. |
If we take workaround option 1 and split processors into their own packages, I would argue that the only OOTB runtime type should be the local runtime type. This would also satisfy those cases where folks want to simply try things out. I believe including any KFP or Airflow in the base installation is contrary to what #2136 is trying to address since Airflow users don't want/need KFP and vice versa. If we take workaround option 2 (which seems more attenable and less "expensive" maintenance-wise), then the responsibility is on us to operate with no platform-specific packages and, more importantly, defer the loading of the processor entrypoints until necessary. One approach that we could use here is to add code to the |
@lresende and my responses crossed, but it looks like we're suggesting similar approaches. |
65dd225
to
9e16d98
Compare
try: | ||
# Check to see if Airflow package is installed, since we do not have any dependencies | ||
# on any Airflow package, we use GitHub as our canary | ||
from github import Github # noqa |
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.
I don't think this will be sufficient. Won't we still need the ability for the user to physically indicate their intentions? Like pip install elyra[kfp]
or pip install elyra[airflow]
.
Do we need a dummy airflow package that we could use (and install via the previous example) to make the user's intention known to server code? This package, while on PyPi and Conda-forge, would likely never have to change, so I'm not sure there would be much of a burden other than its initial creation.
Then elyra[kfp]
would include the KFP sdk, elyra[kfp-tekon]
would include elyra[kfp] + kfp-tekton
and elyra[airflow]
would include this dummy airflow placeholder as its dependency. It could even have a dependency on GitHub, if we needed that (and elyra[gitlab]
could include elyra[airflow] + gitlab
if we wanted that package tied to airflow.
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.
I agree. There may be other reasons a package (like github) might be installed. For example, once the community provides a component catalog connector that relies on the package.
This PR should address the question raised in #2089 |
Instead of a dummy package, would it be possible to create a package for each runtime that serves a purpose, such as identifies the runtime capabilities? This way we would be more consistent across runtimes. |
That works too. We only need something for airflow, but could maybe have something relative to kfp as well. If so, I think these packages would need to be fairly static so as to prevent the need from having regular releases, etc. Do you have an idea of how "runtime capabilities" would be expressed across the various runtimes? What are examples of such? And these capabilities would need to be included in "true" BYO runtime implementations as well. |
For what I was considering that would be the case. I'm also thinking about a potential future 'local' runtime here, which doesn't necessarily have any third party dependencies.
Focusing on the 'what' might be useful to encode, I was thinking this would include whether a runtime supports certain runtime features such as:
The UI could use those hints to disable or hide things, like this PR proposes to do for VPE launcher tiles. |
What changes were proposed in this pull request?
Enables the ability to install selective packages/groups in order to either include or exclude certain runtimes and their dependencies.
How was this pull request tested?
Developer's Certificate of Origin 1.1