-
Notifications
You must be signed in to change notification settings - Fork 348
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
Pipeline Editor: Allow for configuration of shared memory size #2942
Pipeline Editor: Allow for configuration of shared memory size #2942
Conversation
Thanks for making a pull request to Elyra! To try out this branch on binder, follow this link: |
@@ -40,6 +40,7 @@ | |||
from elyra.metadata.manager import MetadataManager | |||
from elyra.pipeline.component import Component | |||
from elyra.pipeline.component_catalog import ComponentCache | |||
from elyra.pipeline.component_parameter import CustomSharedMemorySize |
Check notice
Code scanning / CodeQL
Cyclic import
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
In today's community meeting it was brought up that giving the user the option to choose from multiple units might be complicating things. While I agree that it might not be necessary to support Gi/Mi and G/M by choosing a basis that is applied to all memory units in Elyra, there would benefits to giving the user the option to choose a unit (e.g GB/MB ignoring for now what basis is used) if the units are supposed to be internally interpreted using power of two instead of power of ten. It's not clear to me which basis is used today for the generic components RAM resources setting. The implementation for the KFP and Airflow processor appear to be in disagreement, with the KFP processor applying the G (power of ten-based) suffix, whereas the Airflow processor applying none (which may or may not be a bug). Anybody have insights whether G or Gi is the "correct" one? |
Follow-up required: When I merged the |
This comment was marked as resolved.
This comment was marked as resolved.
b91060f
to
d9b0b25
Compare
Co-authored-by: Alan Chin <[email protected]>
…tzler/elyra into add-shared-mem-configuration-support
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
Requires #2957RESOLVEDExtend the pipeline editor to allow for setting a custom shared memory size in generic and custom components. A pipeline default can be configured in the pipeline properties. Support is limited to Kubeflow Pipelines and Apache Airflow.
To define a custom size the user needs to specify a value other than zero. Zero implies that the Kubernetes default is used, which is 64Mi.
When a custom shared memory size is configured, Elyra adds a volume and volume mount to the pod, as shown here for Kubeflow Pipelines:
Closes #2838
TODO:
What changes were proposed in this pull request?
How was this pull request tested?
pipeline defaults:
generic nodes (KFP and Airflow):
custom nodes (KFP and Airflow):
Developer's Certificate of Origin 1.1