-
-
Notifications
You must be signed in to change notification settings - Fork 332
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
Implement roles.delete
, databases.drop
& databases.configured.disconnect
endpoints
#3858
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I have a couple comments that require discussions, but no changes needed in this PR.
""" | ||
Drop a database from the server. | ||
|
||
Args: | ||
database_oid: The OID of the database to delete on the database. | ||
database_id: The Django id of the database to connect to. | ||
""" | ||
user = kwargs.get(REQUEST_KEY).user | ||
with connect(database_id, user) as conn: | ||
drop_database(database_oid, conn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking more about this and we should only be providing delete
for databases created in the internal server. Only Mathesar admins should be able to perform this.
When the user requests a delete, these conditions should match:
- The user should be a Mathesar admin
- The database should be present within the internal db server
- The collaborator must use a role that owns the database
And then, in a single transaction:
- connect to the database that's about to be deleted
- change ownership to the internal db role that we use in mathesar_django
- disconnect
- connect to the internal db server with the internal db role
- drop the database
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing needed in this PR, we can do this for beta.
database_qs = Database.objects.get(id=database_id) | ||
database_qs.delete() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to discuss on how to remove the Mathesar schemas. This requires a broader discussion on how we'll retain/request the user for the role & password for the role that owns the schemas.
We can get back to this before beta.
Fixes #3772
Fixes #3841
Fixes #3842
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin