Skip to content
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

Add image_pull_secrets paameter to add_settings_to_comp for kfp v2 #915

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

revit13
Copy link
Collaborator

@revit13 revit13 commented Jan 6, 2025

Why are these changes needed?

To address #914 a new param was added to add_settings_to_component function.

To pass the secret name the following param should be added to add_settings_to_component call:

ComponentUtils.add_settings_to_component(compute_exec_params, ONE_HOUR_SEC * 2, image_pull_secrets=["secret-name"])

Note: image_pull_secrets could not be a pipeline input parameter because they are of type PipelineParameterChannel so we get the following error when trying to use them:

  File "kfp/kfp_support_lib/kfp_v2_workflow_support/src/workflow_support/compile_utils/component.py", line 99, in add_settings_to_component
    if len(image_pull_secrets) != 0:
TypeError: object of type 'PipelineParameterChannel' has no len()

Related issue number (if any).

#914

@revit13 revit13 requested a review from roytman January 6, 2025 07:13
@@ -92,6 +94,10 @@ def _add_node_selector() -> None:
task.set_caching_options(enable_caching=cache_strategy)
# image pull policy
kubernetes.set_image_pull_policy(task, image_pull_policy)
# image pull secret can only be set at the component level in kfp v2
# see: https://github.com/kubeflow/pipelines/issues/11498
if image_pull_secret != "" and len(image_pull_secret) != 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one of the checks is redundant

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Thanks

Signed-off-by: Revital Sur <[email protected]>
@roytman roytman merged commit 80e0374 into IBM:dev Jan 6, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants