refactor: clean up deployer db resources, delete after sync to r-r, delete secrets command #1376
Conversation
jonaro00
force-pushed
the
clean-up-deployer-db
branch
from
f32fe97 to
c1abbe3
Compare
jonaro00
changed the title
| @@ -119,12 +119,6 @@ pub enum Command { | |||
| /// Manage resources of a Shuttle project | |||
| #[command(subcommand)] | |||
| Resource(ResourceCommand), | |||
| /// Manage secrets for this Shuttle service | |||
| Secrets { | |||
There was a problem hiding this comment.
suggestion(non-blocking): And actually, are we sure we want to remove this command ? I know we're treating secrets as a resources now, but isn't that an implementation detail ? We could instead ask the r-r for just the secrets.
There was a problem hiding this comment.
Could do that, yeah. I think it's more clear if they are only in one place where they currently logically belong. This command has been broken for quite some time.
There was a problem hiding this comment.
I think it is worth simplifying this and removing the cmd, given the costs coming from maintaining, bugs fixing, and the support specifically for this cmd (and maybe feature requests?).
However, it is worth saying we remove it without a deprecation notice, and backward compat support e.g. previous versions of cargo-shuttle can call secrets cmd on a recent deployer, and I think it will fail, right? We would need to instruct users to use resource list instead, which shouldn't be that bad.
Also, it is worth saying secret retrieval is not extremely useful, given we're not displaying the actual values, so having a dedicated command just for them doesn't make sense to me. Down the line, secrets displaying (along other resources displaying) should be further refined to maybe display hints and data about when it was last modified (and possibly accessed?), but still, not sure if it is worth a dedicated cmd.
There was a problem hiding this comment.
It seems to me like the command is currently broken, like Johan mentioned, so I think we can safely remove it. 😄
There was a problem hiding this comment.
My idea was more to keep the secrets part but in a workable state :p But fine if you think resources make more sense :)
My test with inserting an old record mimics the upgrade from an old version (based on what I observed during support week). I would have to go back several months to do it "naturally". To me, this is sufficient. |
Description of change
secretsCLI command since it only targeted the deployer cache. The new way is to useresource listHow has this been tested? (if applicable)
Tested with inserting a resource in deployer, and the resource list operation correctly synced them up to r-r. The project could then be deleted without hassle.