-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
Connections API #3309
Connections API #3309
Conversation
@Anish9901 Please review, but don't merge until @seancolsen makes the required front end changes. |
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 great @mathemancer! I've addressed a small bug please take a look.
@@ -23,33 +23,10 @@ def get_supported_types_url(self, obj): | |||
if isinstance(obj, Database) and not self.partial: | |||
# Only get records if we are serializing an existing table | |||
request = self.context['request'] | |||
return request.build_absolute_uri(reverse('database-types', kwargs={'pk': obj.pk})) | |||
return request.build_absolute_uri(reverse('connection-types', kwargs={'pk': obj.pk})) |
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.
There is a minor bug here, the uri formed will always be namespaced under ui/
as the basename
is same for both the db
& ui
route. Not sure how significant it is for the frontend though.
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.
Hmmm. I'll experiment with it a bit.
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.
@Anish9901 I poked around and found that the behavior's the same as before. I agree that this is a bit of a bug, but it's apparently harmless. Given that we're going to be completely overhauling the API structure in the near future, I want to avoid fixing it for now.
I'm taking over the frontend work from @seancolsen for changes needed in this PR. |
@mathemancer @Anish9901 I've fixed the breaking changes on the frontend, except the previous 'Database Connections' settings page which will be rewritten as per the new spec. This PR can be merged when ready. |
Fixes #3291
This PR is an initial iteration towards replacing the "Database" concept in our codebase with a "DB Connection" (usually just "Connection") concept.
Technical details
The following endpoints are changed:
/api/ui/v0/databases/*
->/api/ui/v0/connections/*
/api/db/v0/databases/*
->/api/db/v0/connections/*
Note that as indicated by the wildcard, all endpoints under these namespaces are affected.
Changes to keys in JSON representations of connections (orig databases):
name
->nickname
db_name
->database
deleted
-> (removed)editable
-> (removed)error
-> (removed)Changes to common data JSON keys:
.current_db
->.current_db_connection
.databases
->.connections
.databases.[i].name ->
.connections[0].nickname`.databases.[i].db_name ->
.connections[0].database`.internal_database
->.internal_db_connection
.internal_database.db_name
->.internal_db_connection.database
Note that in order to keep this PR small, the Schema model refers to its
database
rather thanconnection
in the API and common data.Behavior changes:
When creating or updating a connection (formerly database) via POST or PATCH, we no longer test the actual connection or install Mathesar on the DB. That will be handled by a subsequent PR on the RPC endpoint.
Regarding
MATHESAR_DATABASES
:I left that variable for now, since removing it would require rewriting major pieces of our back end test suite. However, I don't think it will affect much. I think we should remove that variable from our docs and examples, except for the dev setup, where having a preinstalled DB automatically created is convenient.
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin