PsqlDosMigrator: Remove hardcoding of table name in database reset
#5781
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sphuber
force-pushed
the
fix/5769/psql-dos-clear-table-name-reflection
branch
5 times, most recently
from
e90fb59 to
5c31ef4
Compare
sphuber
force-pushed
the
fix/5769/psql-dos-clear-table-name-reflection
branch
2 times, most recently
from
44217d3 to
28e60df
Compare
|
@ltalirz this is ready for review |
ltalirz
reviewed
Nov 23, 2022
| with self.transaction() as session: | ||
|
|
||
| # Close the session otherwise the ``delete_tables`` call will hang as there will be an open connection | ||
| # to the PostgreSQL server and it will block the deletion and the command will hang. |
There was a problem hiding this comment.
I understand this measure prevents connections opened by this particular process from interfering, but I guess there could be other connections open to the DB (daemon / REST API / verdi shell / ...)?
Is there a way we could detect this and exit with an error message instead of hanging?
There was a problem hiding this comment.
Not sure to be honest. Not sure if sqlalchemy provides an API to get all open connection to a database, even those that are managed by it. I doubt it. We would probably have to go straight to psycopg and directly execute a postgres command. Anyway, for now, this method is only being called during unit testing, so it is not that likely.
The `PsqlDosMigrator.reset_database` and `PsqlDosBackend._clear` methods both deleted (almost) all database tables. The tables to delete were hard-coded in a list by each method. The `delete_tables` method is added to the `PsqlDosMigrator` class which will delete all tables of the schema which are determined dynamically by reflecting the metadata schema. The `exclude_tables` argument can be used to define table names to exclude. The new method is used by both classes in order to remove the hard-coding of table names.
sphuber
force-pushed
the
fix/5769/psql-dos-clear-table-name-reflection
branch
from
28e60df to
ad1250c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5769
The
PsqlDosMigrator.reset_databaseandPsqlDosBackend._clearmethodsboth deleted (almost) all database tables. The tables to delete were
hard-coded in a list by each method.
The
delete_tablesmethod is added to thePsqlDosMigratorclass whichwill delete all tables of the schema which are determined dynamically by
reflecting the metadata schema. The
exclude_tablesargument can beused to define table names to exclude.
The new method is used by both classes in order to remove the
hard-coding of table names.