-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
feat/Include drivers for Postgres and MySQL/MariaDB by default #5452
Conversation
To help me understand why this is useful: So your workflow would be to install the (updated) Docker image, then use the new Python modules to do what? Wouldn't you need to recompile your own Docker images anyways, if you would add Python scripts allowing you to store the data to your remote DBs? |
No these are not for custom scripts, but to enable storing of trade info into a centralised DB vs a SQLite file (cf. https://hummingbot.org/global-configs/external-db/) out of the box. If you are running 20+ docker containers and each of them do frequent trades, SqlAlchemy's ORM via SQLite isn't exactly the most performant out there (cf. https://stackoverflow.com/questions/11769366/why-is-sqlalchemy-insert-with-sqlite-25-times-slower-than-using-sqlite3-directly), and you will end up with lots of disk thrashing. It's also useful to have a central location to do analysis of trades instead of having to get data from individual SQLite file. |
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.
It does not make sense to add more dependencies to libraries for particular DBs. Each user should configure that in their own installation, and it should not be part of the official repository.
-- Reviewed in representation of Hummingbot Foundation Technical DAO
But it makes sense to only have dependency for SQLite built-in? 🤷♂️ Joke aside, would it be better to say, have a different environment.yml file only for the purpose of building a docker image so that it can run out-of-the-box with a db driver other than SQLite? Because when running a cluster of docker containers and all of them write to different SQlite files at the same time, it really cause problems with disk thrashing (backlog disk I/O leading to stalled instance, leading to high memory consumption). |
Or perhaps there is a better way to do it so that the official docker image we pull can already support other RDBMS? |
@cheemeng @aarmoa Mike opened a discussion in CommonWealth for this topic. This is the link: https://commonwealth.im/hummingbot-foundation/discussion/6013-support-for-external-dbs |
Closing PR, snapshot vote: failed |
Current docker image only includes the driver for SQLite.
I believe those of us running many instance of Hummingbot, or want a centralised db storage (for post-trade analytical purposes) would prefer logging trades in a different RDBMS such as PostgreSQL or MySQL/MariaDB. However, this means we would need to build the docker image ourselves every time a new release of Hummingbot is out, in order to work with a non-SQLite database.