search

Chaosthebot / Chaos

A social coding experiment that updates its own code democratically.
http://chaosthebot.com
MIT License
2.44k stars 210 forks source link

Database abstraction layer (mysql implementation) #461

Closed hongaar closed 7 years ago

hongaar commented 7 years ago

Includes #456 Based on #381

rudehn commented 7 years ago

The mysql implementation will return a dict cursor instead of a tuple cursor. Might want to standardize that

hongaar commented 7 years ago

@rudehn 👍 see https://github.com/chaosbot/Chaos/pull/456/commits/a1038b94686b04199f1b62eed193e44c292350be

hongaar commented 7 years ago

@PlasmaPower I'd prefer sqlite for now, because it's much faster and more portable for simply storing some simple state records.

@md678685 With MySQL you'll also have to wait for locks; the operations are just running in another process.

And with MySQL we'll get another server to monitor and another dependency to manage (plus a whole bunch of memory goes to waste).

chaosbot commented 7 years ago

:ok_woman: PR passed with a vote of 10 for and 0 against, a weighted total of 9.5 and a threshold of 6.5, and a current meritocracy review.

See merge-commit 8e9454aac0ccdbc047faf3762461eab5003ef911 for more details.

PlasmaPower commented 7 years ago

@hongaar How would you recommend I approach creating the table? I could do it with the sqlite CLI and the migrations tool.

rudehn commented 7 years ago

@PlasmaPower Just a heads up, but I'm going to start working on integrating peewee so we can abstract out the db type from our queries. Peewee supports both mysql and sqlite.

If you'd like to take a look into it, you're more than welcome. I think having an ORM would be useful.

PlasmaPower commented 7 years ago

@rudehn Okay, I might wait for that. Will you be introducing a migration strategy? It looks like that has functions for doing the migrations themselves, but not versioning or similar.

rudehn commented 7 years ago

I envisioned using the migrations tool and peewee's migration functions. I don't see any versioning either. Is that something we are wanting to do?

PlasmaPower commented 7 years ago

@rudehn I guess the existing tools will work, it's just that they're shell based. I forgot that we could run a separate Python script from them.

rudehn commented 7 years ago

You can go ahead and setup a python migration script to get started.

PlasmaPower commented 7 years ago

@rudehn there's a problem - you can't import lib.db from the a script in the migrations directory.

rudehn commented 7 years ago

maybe we need to add the root level to the path? Try

sys.path.insert(...)

PlasmaPower commented 7 years ago

That might work, but it's pretty hacky.