pipelines
pipelines copied to clipboard
WIP: Add an env var and CLI option to disable default caching
Description of your changes: Add an env var and CLI option to disable default caching, such that the CLI flag setting will take precedence over the environment variable setting. Here's how it works:
If the --disable-caching flag is provided when running the command, it directly controls the disable_caching value.
If the flag is set (i.e., --disable-caching is used), disable_caching will be True.
If the flag is not set, disable_caching will be None.
The environment variable KFP_DISABLE_CACHING is only used if the disable_caching flag is not provided.
Example Scenarios
Scenario 1: CLI Flag is Provided
kfp dsl compile --py cache.py --output output.yaml --disable-caching
Result: Caching is disabled, regardless of the environment variable.
Scenario 2: CLI Flag is Not Provided, Environment Variable is Set
export KFP_DISABLE_CACHING=true && kfp dsl compile --py cache.py --output output.yaml
Result: Caching is disabled because the environment variable is set to 'true'.
Scenario 3: Neither CLI Flag Nor Environment Variable is Set
kfp dsl compile --py cache.py --output output.yaml
Result: Caching is enabled, as neither the flag nor the environment variable is set to disable it.
This logic provides flexibility while ensuring that the user’s explicit command-line instructions (the CLI flag) take precedence over environment settings.
Checklist:
- [x] The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign connor-mccarthy for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Ready for review. cc: @gregsheremeta @zijianjoy @chensun
/retest test-run-all-gcpc-modules
@DharmitD: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:
/test kubeflow-pipelines-integration-v2/test test-run-all-gcpc-modules
Use /test all to run the following jobs that were automatically triggered:
test-run-all-gcpc-modules
In response to this:
/retest test-run-all-gcpc-modules
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.
Environment variable values should align with the conventions used by other Python libraries. Typically, 0 is used to represent false, while any other integer is treated as true. Although some approaches allow for more variation in representing true, such as accepting [1, y, yes, true], it's best to maintain consistency by using only 0 for false and any other integer for true.
To @diegolovison 's point, I also noticed this in the compiler today. There's a strong argument to be made for keeping the user interface consistent.
https://github.com/kubeflow/pipelines/blob/1c7954de261b22fa93905cf11b6b280fdf19c320/sdk/python/kfp/cli/compile_.py#L131-L135
@DharmitD , how difficult would it be to rework this using a Feature flag?
Closing as this has been covered in https://github.com/kubeflow/pipelines/pull/11222