SQLAlchemy rewrite

[RustyBower]

No description provided.

[RustyBower]

I wasn't sure what to do with the requirements file, so please test this out and let me know what I should pin sopel versions to...

This should work with any version of sopel, but please confirm my suspicions :)

[dgw]

tl;dr: sopel.db.engine isn't technically part of the API, and shouldn't be used directly by plugins.

[dgw]

This should work with any version of sopel

I doubt it. Sopel before 7.0 (current master there) uses SQLite3 directly, not SQLAlchemy, so a lot of this stuff probably won't work properly. Just bump the requirements to sopel>=7.0,<8 like you did for the YouTube plugin.

[RustyBower]

Refactored all the database connectivity things.

[dgw]

I see some issues with consistency. Seems to be very confusing naming variables engine if they actually hold Connection objects?

Also, the repeated try/except for getting a DB session could probably stand to get abstracted out into a function. Maybe a function that takes a Sopel (bot) object and returns the session (or raises an exception if it can't get one)?

I was really hoping to avoid importing SQLAlchemy into this project, but I get the impression that we have to unless the DB object API in Sopel itself gets fleshed out a ton.

[dgw]

Sometimes you use engine, sometimes engine.engine. Are you sure about this code?

[RustyBower]

Weirdly it works both ways lol

[RustyBower]

The colors don't translate into GH comments, but I promise they still look fabulous.

05:24:24 <sopel> Successfully enabled listening for rustybower/test's events in #bottest.
05:24:24 <sopel> Great! Please allow me to create my webhook by authorizing via this link: https://git.io/Jea5L
05:24:25 <sopel> Once that webhook is successfully created, I'll post a message in here. Give me about a minute or so to set it up after you authorize. You can configure the colors that I use to display webhooks with .gh-hook-color
05:25:30 <sopel> [test] RustyBower: It's not fully shipped until it's fast. (Your webhook is now enabled)
05:27:51 <sopel> [test] RustyBower pushed 1 new commit to master https://git.io/JeVu4
05:27:52 <sopel> test/master 335e2ae Rusty Bower: testing
05:28:00 <@RustyCloud> .gh-hook-color rustybower/test 1 2 3 4 5 6
05:28:00 <sopel> [rustybower/test] Example name: RustyCloud tag: tag commit: c0mm17 branch: master url: https://git.io/
05:28:21 <sopel> [test] RustyBower pushed 1 new commit to master https://git.io/JeVuB
05:28:21 <sopel> test/master 441cae1 Rusty Bower: testing

[dgw]

Why is the same identical function defined twice? Not to nitpick too much, but that seems very silly. Even sillier than the fact that setup_webhook() still has dedicated DB-connection code… which is kind of on me, because I mentioned sessions and that doesn't need one. 😆

The colors don't translate into GH comments

Indeed, the relative lack of formatting options here vs. on IRC caught me out when trying to write nice PR descriptions for additions to sopel.tools.formatting a while back.

[dgw]

Not doing a full-on review, but there's one thing I noticed (line-noted). Also, should this not use a similar approach to Sopel's own DB class for thread-safety? That is, with regard to the scoped session management approach you proposed in sopel-irc/sopel#1755.

[dgw]

Isn't this pulled in from .utils now?

[dgw]

It's no longer necessary to make any changes just to make this plugin keep working—tweaks to Sopel's database API mean this and other plugins that use direct DB access with Sopel 6.x will continue to work with 7.x if the default SQLite database backend is in use.

That said, we should still migrate onto SQLAlchemy eventually. It'll give this plugin compatibility with more database types, and as more upstream users take advantage of the new supported backends the restriction to using this plugin with SQLite will become more onerous.