Fix trusted deprecation warning. #1412
Conversation
| @@ -1087,8 +1087,8 @@ def run_python(script, target_directory): | |||
| type=bool, | |||
| is_flag=True, | |||
| required=False, | |||
| default=False, | |||
| help="[DEPRECATED]Trust truss with hosted secrets.", | |||
| default=None, | |||
There was a problem hiding this comment.
Is it weird to make a bool flag have a possible value of None? Instead could we leave the default as False, and then log below if trusted?
There was a problem hiding this comment.
A bit weird, it's "implicit" optional which is against strict typing. Unfortunately Optional[bool] does not work with click.option.
However, I find it more weird to set the default to False while the effective behavior is actually True.
None seems a good value for indicating "unspecified" which seems aligend with deprecated, even if there's a typing nit here. Alternatively we could set the default to True, then the warning case would never be triggered.
There was a problem hiding this comment.
I think it's good to leave the warning in case people are explicitly passing --trusted as well (and no longer need to). I'm sold that None is better than a confusing default, and the typing of the actual python function is more clear!
marius-baseten
force-pushed
the
marius/trusted-fix
branch
from
7092f83 to
9c389ee
Compare
🚀 What
See #1405 (comment)
💻 How
🔬 Testing