Remove soft-deletion pattern

kolide / fleet

A flexible control server for osquery fleets

https://kolide.com/fleet

MIT License

1.1k stars 260 forks source link

Remove soft-deletion pattern #2327

Closed nyanshak closed 4 years ago

nyanshak commented 4 years ago

Perform migration to delete any entries with deleted set, and subsequently drop columns deleted and deleted_at.
Remove deleted and deleted_at references.

nyanshak commented 4 years ago

Addresses #2146

nyanshak commented 4 years ago

@zwass I took a stab at solving #2146 here. It's still a bit of a work in progress, but I could use some help with the migrations since I haven't worked with that before.

I'm following the instructions in docs/development/database-migrations.md, but having a bit of trouble.

When I run ./build/fleet prepare db, my migration file is called (added debug prints in init), but the Up function doesn't seem to ever be called.

Could you take a look and see if I'm doing something wrong?

nyanshak commented 4 years ago

One of the test failures is when deleting hosts. There is an existing test for testIdempotentDeleteHost that creates a host and tries to delete it twice. In the 'soft-deleted' pattern, this was fine (just set deleted to true each time). There are lots of places that call deleteEntity but DeleteHost was the only one with a test expecting this to be idempotent.

I'm not sure what the reasoning is behind that, but I'm going to put up a commit that removes this test, as deleting is not actually idempotent any more. Give me a shout if this doesn't work.

zwass commented 4 years ago

I am totally on board with removing tests that no longer make sense. Thank you. I will give this some more review and testing when I have a chance.