Skip to content

Conversation

@sduvvuri1603
Copy link
Contributor

@sduvvuri1603 sduvvuri1603 commented Oct 29, 2025

Description of changes

  • Follow-up to #12377 to ensure the optional Kubernetes runtime test actually exercises the driver logic.
  • Collapse missing_kubernetes_optional_inputs_pipeline to a single log_message task and apply every K8s helper directly on that task.
  • Add optional_image_pull_secret_name so image pull secrets participate in the skip logic.
  • Remove the dsl.If guards; the helpers now see either real values or null, letting the driver hit both the “patch applied” path and the ErrResolvedParameterNull skip path.

@google-oss-prow
Copy link

Hi @sduvvuri1603. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@github-actions
Copy link

🚫 This command cannot be processed. Only organization members or owners can use the commands.

@mprahl
Copy link
Collaborator

mprahl commented Oct 29, 2025

/ok-to-test

@alyssacgoins
Copy link
Contributor

/ok-to-test

@alyssacgoins
Copy link
Contributor

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Oct 31, 2025
@sduvvuri1603 sduvvuri1603 force-pushed the fix/optional-k8s-followup branch from 04e7bd3 to 5cbccda Compare November 4, 2025 15:11
@google-oss-prow google-oss-prow bot removed the lgtm label Nov 4, 2025
@sduvvuri1603 sduvvuri1603 marked this pull request as ready for review November 4, 2025 15:26
@sduvvuri1603 sduvvuri1603 marked this pull request as draft November 4, 2025 15:26
@VaniHaripriya
Copy link
Contributor

@sduvvuri1603 Please add the TestData entry for new Pipeline here- https://github.com/kubeflow/pipelines/blob/master/sdk/python/test/compilation/pipeline_compilation_test.py#L271.

@sduvvuri1603 sduvvuri1603 force-pushed the fix/optional-k8s-followup branch 2 times, most recently from 12d70b9 to b5d93cb Compare November 4, 2025 23:05
@sduvvuri1603 sduvvuri1603 force-pushed the fix/optional-k8s-followup branch from b5d93cb to 8c54f94 Compare November 4, 2025 23:37
@VaniHaripriya
Copy link
Contributor

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Nov 5, 2025
@mprahl mprahl marked this pull request as ready for review November 5, 2025 15:48
with dsl.If(enable_affinity == True):
affinity_task = log_message(message="node affinity applied")
affinity_task.set_caching_options(enable_caching=False)
if optional_affinity is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can't use conditionals like this in a dsl.pipeline because this code executes at compile time, not runtime.

Just remove the if statements. We also want to always apply the unset Kubernetes configuration to test your new code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad. Removed the if statements and pushed the updated code changes.

Copy link
Collaborator

@mprahl mprahl left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@mprahl mprahl marked this pull request as ready for review November 5, 2025 19:02
@google-oss-prow google-oss-prow bot requested a review from zazulam November 5, 2025 19:02
@mprahl
Copy link
Collaborator

mprahl commented Nov 5, 2025

/ok-to-test

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mprahl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 36c8437 into kubeflow:master Nov 5, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants