search

laminlabs / lamindb-setup

Setup & configure LaminDB.
Apache License 2.0
4 stars 2 forks source link

🔒 Raise error if migration is done by wrong user #813

Open fredericenard opened 1 month ago

github-actions[bot] commented 1 month ago

🚀 Deployed on https://66afacc643ec4dc2edf6f5a8--lamindb-setup-htry.netlify.app

falexwolf commented 1 month ago

This goes in a good direction, but I don't think we should merge anything until we triple checked that we have a final naming scheme.

Also, we already have this check running before migration deployment. It's only based on the permission table but of course these things should be in sync. Let's see whether some of the logic that this is in sync is client side or not. As usual, we won't be able to change it if it's client-side.

https://github.com/laminlabs/lamindb-setup/blob/93a9fbf0b7dfa10ba36707fa51a2f6b4df105b76/lamindb_setup/_migrate.py#L84-L93

fredericenard commented 1 month ago

I don't think we should merge anything until we triple checked that we have a final naming scheme.

Yes I agree,

Also, we already have this check running before migration deployment. It's only based on the permission table but of course these things should be in sync.

There is no way to be 100% the right connection string will be used

Maybe a better approach is having a external process ensuring all tables are owned by the root user 🤔

I see several ways to do that, one pretty simple would be to use a postgres trigger, but we would still have the problem with logic living client side.

Another way is to have a cron performing regular checks for this. This could be part of a bigger piece of logic including health checks for the instance and storage diagnostic (to verify policy is properly attached).