Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rename delete to stop #619

Merged
merged 4 commits into from
Feb 9, 2023

Conversation

jdrouet
Copy link
Contributor

@jdrouet jdrouet commented Feb 8, 2023

According to this comment, it's not necessary to remove the service from the database and we only need to stop it.
So, in the deployer, we can rename the delete_service function to stop_service and just remove the persistence line for dropping the service in database.

Considering this endpoint no longer deletes the service but just stops it, we can rename the delete command to stop in cargo-shuttle.

This should fix #561

@oddgrd
Copy link
Contributor

oddgrd commented Feb 9, 2023

Thanks @jdrouet! This looks like it's nearly there, but won't we still get an FK error from calling delete_deployments_by_service_id?

@jdrouet
Copy link
Contributor Author

jdrouet commented Feb 9, 2023

@oddgrd good point, thank you for noticing it 😉

Copy link
Contributor

@oddgrd oddgrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! One last thing: I think we can safely remove the delete_deployments_by_service_id function, I don't think it's used anywhere else.

@jdrouet
Copy link
Contributor Author

jdrouet commented Feb 9, 2023

LGTM, thanks! One last thing: I think we can safely remove the delete_deployments_by_service_id function, I don't think it's used anywhere else.

Sure, I'm used to have a clippy job that would break on the CI for this kind of issues so I tend to forget to check 😅

According to [this
comment](shuttle-hq#616 (comment)),
it's not necessary to remove the service from the database and we only
need to stop it. So we can rename the `delete_service` function to
`stop_service` and just remove the persistence line for dropping the
service in database.

Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
Considering this endpoint no longer deletes the service but just stops
it, we can rename the delete command to stop.

Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
@oddgrd oddgrd merged commit a0b412e into shuttle-hq:main Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: foreign key constraint failed when running cargo shuttle delete
2 participants