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

Add migration to mark orphaned environments deleted #3636

Merged
merged 4 commits into from
Jan 16, 2024

Conversation

bomoko
Copy link
Contributor

@bomoko bomoko commented Jan 8, 2024

As discussed in #3630 there are cases in previous versions of Lagoon where projects have been deleted and have had their environments orphaned without the (the envs) having their deleted field set to a non-zero/default value.

This PR introduces a migration to set the deleted field to the current date/time to mark an environment as having been deleted.

Note: there is no "down" specified for this migration - I wasn't sure what we wanted to do here. It's obviously not best to just simply reset all orphaned items deleted field to 0, since that's not the state of the DB pre migration (or, isn't guaranteed to be).

One option, I suppose, would be to set the date/time to something very specific - like "0000-00-01 11:11:11" -- assuming that this value wouldn't arise "naturally" -- and then have the down migration target environments with that date/time?
Thoughts?

General Checklist

  • Affected Issues have been mentioned in the Closing issues section
  • Documentation has been written/updated
  • PR title is ready for inclusion in changelog

Database Migrations

  • If your PR contains a database migation, it MUST be the latest in date order alphabetically

@bomoko bomoko requested a review from shreddedbacon January 8, 2024 20:04
Copy link
Member

@shreddedbacon shreddedbacon left a comment

Choose a reason for hiding this comment

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

Awesome :D this will be very nice

Note: there is no "down" specified for this migration - I wasn't sure what we wanted to do here. It's obviously not best to just simply reset all orphaned items deleted field to 0, since that's not the state of the DB pre migration (or, isn't guaranteed to be).
One option, I suppose, would be to set the date/time to something very specific - like "0000-00-01 11:11:11" -- assuming that this value wouldn't arise "naturally" -- and then have the down migration target environments with that date/time?
Thoughts?

I don't think a "down" is required for this particular migration, it is fixing a problem that should have been this way from the start. How would you come back from this, the projects don't exist anymore?

@bomoko
Copy link
Contributor Author

bomoko commented Jan 9, 2024

I don't think a "down" is required for this particular migration, it is fixing a problem that should have been this way from the start. How would you come back from this, the projects don't exist anymore?

I agree, this is what I'm implying as well. Assuming that the "down"'s job is to get the DB in the same state it was in before running the migration up, then essentially you'd be wanting to update those (and only those) items targeted by the up migration and set them back to "0000-00-00 00:00:00", right?
This is why I thought something like setting the deleted datetime to something very specific (and not natural) would be one way of rolling back the change (there are other ways, of course).

I don't think that it's necessary, though - which is why I didn't supply an implementation! I just wanted to not that I didn't, and make sure nobody had any issues with that :)

I say, migrate:up with impunity!

@bomoko bomoko force-pushed the feature/deal_with_orphaned_environments branch from 367a1af to 89a61e5 Compare January 9, 2024 21:52
@bomoko bomoko requested a review from shreddedbacon January 15, 2024 00:07
Copy link
Member

@shreddedbacon shreddedbacon left a comment

Choose a reason for hiding this comment

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

Looks good now

@shreddedbacon shreddedbacon changed the title Adds migration to delete orphaned environments Add migration to delete orphaned environments Jan 16, 2024
@shreddedbacon shreddedbacon changed the title Add migration to delete orphaned environments Add migration to mark orphaned environments deleted Jan 16, 2024
@shreddedbacon shreddedbacon merged commit c44d543 into main Jan 16, 2024
1 check passed
@shreddedbacon shreddedbacon deleted the feature/deal_with_orphaned_environments branch January 16, 2024 02:38
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.

2 participants