yassineaboukir
Database/integration
Database integration in Sublert, through SQLAlchemy.
Using the SQLAlchemy ORM will enable using most kinds of databases with sublert. I have tested with Postgresql, MySQL and Sqlite without any problems.
Requesting feedback and any further suggestions before the Database/integration branch is merged with the base.
Thanks!
Hi,
I'll review it as soon as I can. I appreciate your contribution!
Thank you.
Should this be configured to use SQLite by default? Not everyone will have a postgres database and it would let less-technical users get up and running quickly. Maybe even add an import function to load their old domains.txt files into the new database for users that are upgrading.
There are instructions in the Readme for configuring postgres database. I assumed that the target audience who have gone through the setup instructions (slack, cron) in the medium article to set up this tool will be competent enough to setup postgres or change the database connection string to use sqlite, which is as trivial as changing a single line of code all thanks to the SQLAlchemy ORM.
With regards to importing existing data, good idea. I can implement that post pull as a response to a feature request, or another commit, push pull cycle.
On Fri, Aug 2, 2019, 1:51 AM Dan Nemec [email protected] wrote:
Should this be configured to use SQLite by default? Not everyone will have a postgres database and it would let less-technical users get up and running quickly. Maybe even add an import function to load their old domains.txt files into the new database for users that are upgrading.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/yassineaboukir/sublert/pull/23?email_source=notifications&email_token=ADBRIDGEEAURGE7Y2L3TKG3QCNLHLA5CNFSM4IFR7XD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3L7YSI#issuecomment-517471305, or mute the thread https://github.com/notifications/unsubscribe-auth/ADBRIDDBB7ZRK74X4E226FTQCNLHLANCNFSM4IFR7XDQ .
Hi,
I reviewed and tested it. This looks pretty solid, a bit of changes required but nothing major. I'll work on pushing it this weekend.
Thank you!
Hey man, cheers!. Let me know if anything is needed from my side.
I have made a couple of more commits since last pull request to you, for minor changes like consolidating the newly found domains into a single message, option to specify the DB engine to use, defaulting to sqlite if no DB engine is specified, etc
You can check the branches and commits in my github, if all seems ok to you, I can send pull requests for these.