Skip to content
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

Feature add postgresql support #151

Merged
merged 3 commits into from
Sep 17, 2024

Conversation

TheoPascoli
Copy link
Contributor

Summary:

This pull request adds support for PostgreSQL in addition to the existing MySQL support. To achieve this, a new environment variable sql_db_type has been introduced, allowing the user to choose between PostgreSQL and MySQL. By default, if the sql_db_type variable is not set, MySQL is selected.

Changes Made:

Database Selection:
    Introduced a new environment variable sql_db_type.
    Default behavior remains unchanged with MySQL being the default if sql_db_type is not specified.
Method Optimization:
    Modified the method 'check_sql_available' to improve its efficiency.
Insert Operation Adjustment:
    Changed the method for inserting data into the systems table because INSERT IGNORE is not supported in PostgreSQL. 
    For PostgreSQL, the query now uses INSERT INTO ... ON CONFLICT DO NOTHING.

Please let me know if there are any questions or further modifications required.

Copy link
Collaborator

@JanMaartenvanDoorn JanMaartenvanDoorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks and nice addition to openSTEF! I think many can benefit from postgresql support.

I found a few minor things we can improve. The most important one is that sql_db_type is now defined as a string. As there are only two options and different forms (lower and upper) are used throughout the code I think it is nice to implement is as an enum (https://www.geeksforgeeks.org/enum-in-python/). This way we get a bit more control and validation.

In addition, I see that there are conflicts with the main branch, please resolve these as well.

Let me know if you have questions about this.

openstef_dbc/data_interface.py Outdated Show resolved Hide resolved
openstef_dbc/data_interface.py Outdated Show resolved Hide resolved
Signed-off-by: Theo Pascoli <[email protected]>
@alicecaron alicecaron force-pushed the Feature-add-Postgresql-support branch 4 times, most recently from a339a9c to c4f7656 Compare September 6, 2024 16:05
@alicecaron
Copy link
Collaborator

alicecaron commented Sep 6, 2024

sql_db_type

I have:

  • updated the main branch and rebased on it.
  • created a new Enum: SupportedSqlTypes fot the available sql engines
  • added unit tests

Please let me know if you see anything else to change

@JanMaartenvanDoorn
Copy link
Collaborator

@alicecaron Looks good, I see that black is failing though. You can probably fix this by running black . in the root of the repository and committing the changes.

@alicecaron
Copy link
Collaborator

@alicecaron Looks good, I see that black is failing though. You can probably fix this by running black . in the root of the repository and committing the changes.

Yes! I ran black on a new commit, it should be ok now @JanMaartenvanDoorn

CARON Alice added 2 commits September 12, 2024 10:12
Copy link
Collaborator

@JanMaartenvanDoorn JanMaartenvanDoorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alicecaron Looks good now!

@alicecaron alicecaron merged commit 24f703e into OpenSTEF:main Sep 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants