feat: deprecate "drop credentials" #2823
Conversation
There was a problem hiding this comment.
- how long is our deprecation period: we should note what that is on the ticket and in the code. (it can be the first 0.x.0 release after July 1 2024. (or whatever)).
- will we want to have a period where deprecation warnings receive a special "redirecting" error message? relative to the generic "this syntax isn't supported"
- would it make sense to have a CLI flag, or connection mode that turns deprecations into errors so that people can validate that their applications don't use deprecated methods?
Just a couple of questions and a proposed rename
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct DropCredentialsExec { | ||
| pub catalog_version: u64, | ||
| pub names: Vec<String>, | ||
| pub if_exists: bool, | ||
| /// If true, show a deprecation warning when the operation is executed. | ||
| pub show_deprecation_warning: bool, |
There was a problem hiding this comment.
To my mind, the deprecation is a static/implementation property of the type, and "show deprecation warning" is a property of the session/client and not the physical plan.
Is there a reason that I've missed that means that the display/output of the struct has to be mutable.
I've read this again, and I think my confusion is that this struct is DropCredntials and should be renamed to match the output. I get why this is here, but it feels like it makes it difficult to generalize, and that we'll have to do this work basically every time we want to change syntax or deprecate something.
There was a problem hiding this comment.
yeah I considered renaming it to DropCredential, but didn't want to overcomplicate the PR. Will update with the rename.
|
(the test failure is material to the PR, was checking because of the minio failures we see... it's interesting to me that this only impacted minio. I'm not sure why this is only picked up on minio though) |
closes #2796
Note for reviewers
The deprecation message itself is purposely not tested in the SLT's as it will be removed in a future release.