feat: add parameter cleanup-state to Tekton tasks#677
Conversation
ppitonak
commented
Dec 3, 2025
ppitonak
commented
- add parameter to tasks
- fix parsing or argument (--cleanup-state=false was ignored)
|
Additionally, even default value is ignored when no parameter is provided in shell :( All issues should be solved by this PR. |
| Serverless: viper.IsSet(params.Serverless), | ||
| ForceDestroy: viper.IsSet(params.ForceDestroy), | ||
| CleanupState: viper.IsSet(params.CleanupState), | ||
| CleanupState: viper.GetBool(params.CleanupState), |
There was a problem hiding this comment.
WHy did you change from IsSet to GetBool?
There was a problem hiding this comment.
For reasons explained in description of this PR. IsSet works fine when default value is false (which is all cases in mapt except of cleanup-state). However, when the default value is true:
- if you don't set the param, users expect state to be cleaned up because default value is true, in fact
IsSetreturns false (correctly) so nothing happens - if you set parameter
--cleanup-state=false, IsSet returns true which is correct but not expected - if you set paramter
--cleanup-stateor--cleanup-state=true, it works as expected
There was a problem hiding this comment.
ohh...I did not realize that... I mean...this should not be controlled like that...but changing the description of the flag....
I mean instead of --clean-state (default true...) so there is no way to say false only setting not setting it...we should rename to --keep-state ??
WDYT? so --keep-state default is false...so it will delete it...and if anyone wanna keep it...should add the param??
There was a problem hiding this comment.
Sure, that would be even easier to use. I'll revert the logic.
There was a problem hiding this comment.
Updated but not tested yet
* parameter cleanup-state refactored into keep-state * added to Tekton tasks Signed-off-by: Pavol Pitonak <ppitonak@redhat.com>
ppitonak
force-pushed
the
cleanup_state_tkn
branch
from
dbd9934 to
4612497
Compare
adrianriobo
left a comment
adrianriobo
left a comment
There was a problem hiding this comment.
LGTM leave up to you for rebase and merge once you test it / decide
2 participants
