Skip to content

refactor: migrate database layer from sqlite3 to SQLAlchemy ORM#29

Merged
sin4ch merged 15 commits into
sin4ch:mainfrom
alidauda:sql_postgres
Sep 7, 2025
Merged

refactor: migrate database layer from sqlite3 to SQLAlchemy ORM#29
sin4ch merged 15 commits into
sin4ch:mainfrom
alidauda:sql_postgres

Conversation

@alidauda

@alidauda alidauda commented Sep 6, 2025

Copy link
Copy Markdown
Contributor

Summary

  • Migrated the entire database layer from raw sqlite3 operations to SQLAlchemy ORM
  • Added proper model definitions for Account, WebhookEvent, and Transaction entities
  • Updated all database operations to use consistent SQLAlchemy session management
  • Fixed configuration and dependency issues
    with this changes user can use any database of their choice without extra configuration which means they are allowed to use etheir postgres mysql or sqllite
Screenshot 2025-09-06 224056

…te database connection handling

- Added python-decouple to manage environment variables more effectively.
- Updated database initialization in webhook_server.py to use DATABASE_URL from environment variables.
- Refactored webhook signature verification for improved readability.
- Enhanced error handling in webhook processing.
- Updated test cases to reflect changes in database connection string.
- Updated dependencies in pyproject.toml to include sqlalchemy and python-decouple.
@sin4ch

sin4ch commented Sep 6, 2025

Copy link
Copy Markdown
Owner

This is what I love to see! 🥹🥹🥹🥹🥹

Comment thread mono_banking_mcp/webhook_server.py Outdated
Comment thread mono_banking_mcp/webhook_server.py
@alidauda

alidauda commented Sep 6, 2025

Copy link
Copy Markdown
Contributor Author

Let me know if you need me to change anything

@sin4ch

sin4ch commented Sep 6, 2025

Copy link
Copy Markdown
Owner

Let me know if you need me to change anything

Those commas after the last parameter/arguments in the methods. I don't think they're meant to be that way.

@alidauda

alidauda commented Sep 6, 2025

Copy link
Copy Markdown
Contributor Author

Fixed

@sin4ch

sin4ch commented Sep 7, 2025

Copy link
Copy Markdown
Owner

Fixed

There are still more commas that I referenced in that file.

@alidauda

alidauda commented Sep 7, 2025

Copy link
Copy Markdown
Contributor Author

Fixed

There are still more commas that I referenced in that file.

the comma don't really matter it more of lint prefence

@sin4ch sin4ch added the enhancement New feature or request label Sep 7, 2025
@sin4ch sin4ch assigned sin4ch and alidauda and unassigned sin4ch Sep 7, 2025
@sin4ch

sin4ch commented Sep 7, 2025

Copy link
Copy Markdown
Owner

Hey @alidauda

Please check for merge conflicts.

raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="internal server error"
detail="internal server error",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This comma again... 🥲

That's the end of the argument/parameter definition so there shouldn't be a comma, right?

raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="invalid webhook signature"
detail="invalid webhook signature",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Again... 😪

@@ -1,7 +1,9 @@
"""

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This looks great, except for the commas at the end of the argument definitions for some of the methods you used.

for txn in transactions:
db.delete(txn)

db.commit()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm actually digging this refactor. ORM ftw

.limit(limit)
.all()
)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for this formatting. It seems you ran the lint check🙏🏽

@sin4ch sin4ch self-requested a review September 7, 2025 13:58

@sin4ch sin4ch left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM!

@sin4ch sin4ch merged commit f416d87 into sin4ch:main Sep 7, 2025
2 checks passed
@sin4ch

sin4ch commented Sep 7, 2025

Copy link
Copy Markdown
Owner

Let's go! 🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants