-
Notifications
You must be signed in to change notification settings - Fork 0
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
DB Client #21
DB Client #21
Conversation
From the models, it works with a new DB connection and can initialize the tables, but I'm not sure how it behaves on an already existing database |
Are you talking about the session.add() behaviour for existing tables / users (in this case) here? maybe this doc helps : Session.add() is used to place instances in the session. For transient (i.e. brand new) instances, this will have the effect of an INSERT taking place for those instances upon the next flush. For instances which are persistent (i.e. were loaded by this session), they are already present and do not need to be added. Instances which are detached (i.e. have been removed from a session) may be re-associated with a session using this method |
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.
lgtm
I'm not sure, the test case I feel like is missing would be the following.
this is fine and one of the unit tests checks that this works as expected Though, if I have already previously initialized the application and applied version 1 of the models to the database, but now, version 2 is released and there is a new column to the user, I am not sure that this setup will revise the database in the way that would be expected (which is what we previously used alembic for). Not sure if that makes the mess in my head any clearer? I'll have a review of the documentation and see if I can come up with something, but, yeah, right now, not sure this sufficiently replaces the revisions that are provided by alembic. edit -- did not realise alembic actually allows for an auto generate. So on the client side, we can use the models to auto-generate the relevant alembic revisions, I think this needs to be done client side, to handle any extensions we might independently add to the models. I'll try to encapsulate this in a test and add some documentation. |
Ah I see, yeah for changes to the DB models I think sticking to alembic and auto generate the revisions makes sense. If later revisions of the table add required columns however this might lead to conflicts, if the according data entries to be added/migrated do not have the required new column. But if we keep a simple data model, like the User in here for the first revisions and have according test data, we can add revisions specific to our deployments outside of this repo. |
Updated it and added tests to check for this expected behaviour with alembic. So, testing framework will auto-generate and apply the revisions based on the BASE from the client settings. Test also checks these can be extended and applied in-situ by a user. Should be fine now, just need to document the expectation for how to use it so it's clear! |
Add layout for how to initialize connection to postgresql. Functional for both a live DB connection and also for the tests. The tests require postgresql to be installed where they are running. May need to add to the .devcontainer for the future.