feat(cargo-shuttle): maintain project id when deleting/creating project #2098
Conversation
cargo-shuttle/src/lib.rs
Outdated
| } | ||
|
|
||
| self.ctx.set_project_id(project.id); | ||
| self.ctx.save_local_internal()?; |
There was a problem hiding this comment.
If we are calling project create (which can be called from any dir) from a directory without a rust project, this would write a .shuttle/config.toml file in a location where it is not wanted. We might need a way to save the config only if it existed when trying to load it.
cargo-shuttle/src/lib.rs
Outdated
| let res = client.delete_project(pid).await?.into_inner(); | ||
|
|
||
| self.ctx.remove_project_id(); | ||
| self.ctx.save_local_internal()?; |
There was a problem hiding this comment.
If a --id different from the id in the config file is provided, we don't want to clear the id in the config file.
There was a problem hiding this comment.
@iksuddle Your comment about return type on load_project_id seems to have disappeared for me... Here's my reply anyways:
Only knowing where the loaded id came from is not enough to know if an explicit --id arg is different from the one in the local_internal config file, so it seems like we need to know what was provided in both places.
Maybe adding a self.ctx.linked_project_id() -> Option<&str> (returning the id loaded from the file, if exists) and comparing it to the self.ctx.project_id() (the "resolved" id) will work?
The note about only doing it when the file exists is also valid. The method above could be used to know if the file exists, since the id is the only field in there. It could also be generalised to something like self.ctx.local_internal_exists() -> bool.
There was a problem hiding this comment.
Sorry, I deleted that comment when I realized it wouldn't work.
There seems to be some discrepancy between using --id vs --name.
Currently, if you run shuttle project delete --id 123 in a directory that is not a rust project, you get the error Failed to find a Rust project in this directory ... due to calling project_args.workspace_path() at the top of load_local_internal_config(). The error comes from cargo metadata. So you can't actually delete a project unless you're in a crate.
I'm not sure if this is the desired behavior, but using --name instead doesn't cause this error and results in Project with name '123' not found in your project list.
To be honest, I'm not sure why they are different, since in both load_local_internal_config and get_local_config have workspace_path().unwrap_or(...).
Also I was wondering what exactly should happen? Should both --id and --name delete projects, or neither, or just --name?
There was a problem hiding this comment.
Additionally, I believe project_create would need ctx.project to be Some (by calling load_project_id) so that the config file can be created/updated. This means adding it to the match for commands that require load_project_id.
Edit:
After looking into it some more I realized there's more to consider since the load_project_id function only creates a project for the deploy command, and it would be weird to create a project twice. So I guess we could either set ctx.project before calling save_local_internal in project_create, or change the way create_or_update_ignore_file works.
|
Thanks for all the feedback! I'll aim to fix everything you mentioned soon. |
|
|
||
| pub fn remove_project_id(&mut self) { | ||
| *self.project_internal.as_mut().unwrap().as_mut().unwrap() = | ||
| InternalProjectConfig { id: None }; |
There was a problem hiding this comment.
Maybe it's better to modify the id field in place instead of replacing the config (in case more fields are added). Same is true for set_project_id
Addresses Issue #2055 by updating
.shuttle/config.tomlas follows:idwhen a project is deleted usingshuttle project deleteidwhen a project is created usingshuttle project create --name ...This should prevent potential mismatches and provide a smoother user experience.