db: add methods to delete a channel or nick's key from database

[deathbybandaid]

These are functions I've had for my bot for a long time,

• reset - sets a value to None
• adjust - allows math adjustments to existing values

Reset can likely use a better method like DELETE FROM instead of replacing values with None.

Adjust is something I've been using for a long time now to retrieve a current value and add a positive or negative number to it. Handy for modules that need to update stat counters.

[dgw]

It's easy to 👍 a function to delete values from the database, though I would definitely name the functions delete_{nick|channel}_value() rather than reset (and use a DELETE statement to match the name instead of setting the value to None).

Getting on board with an adjustment function is much harder. Sopel's database doesn't care what you put in it—everything becomes strings under the hood. Because of that, I don't think the db layer is a good place to put manipulator functions. Changing a value in the DB is more of a "program logic" operation, and I say the code to do it should live in the program (read: plugin) that wants to do it.

[Exirel]

I agree with @dgw.

I would like to point out that Sopel's DB interface will be modified for 7.x, by using SQLAlchemy. So if you want to add or change DB-related features, I invite you to either wait for #1446 to be merged, or to base your work on this branch (but that's risky, considering it's a very living branch with rebase every now and then).

[dgw]

Well it's not changing much at this point, but I agree that rebasing onto a branch that has been rebased is annoying.

Shouldn't be long, though!

[deathbybandaid]

I removed the adjust_ and figured out how to properly DELETE via sqlite

[Exirel]

Straightforward, yet naming things is still hard! 😉

Waiting for #1446 to be merged first.

[dgw]

Reminder for myself (and/or the ambitious PR owner) to give the function names and docstrings a good once-over after this is reworked on top of SQLAlchemy/#1446. (I still prefer _value to _key in the names, and "delete" instead of "reset" in the docs.)

[dgw]

Ready to rework, @deathbybandaid. #1446 just merged in. ☺

[deathbybandaid]

Assuming #1603 gets fixed, my research shows that this is how to delete a value via sqlalchemy. This will need some testing pending that fix.

[dgw]

I'm pretty sure you want to call session.delete(result) (not result.value), but I'd get confirmation from @RustyBower on that one since he has much more SQLAlchemy experience than any of the rest of us.

Going off of https://stackoverflow.com/a/26643978/5991 fwiw

[RustyBower]

Yeah it's definitely session.delete(result), otherwise you're gonna run into errors.

[deathbybandaid]

Applied suggestions

[Exirel]

LGTM

[dgw]

Closed/reopened to kick Scrutinizer in the pants (it errored during the last inspection and I didn't notice until this weekend). I guess we'll also get a new Travis build out of it, so yay?

[dgw]

One wee copy-and-paste leftover to fix, and then I think this is ready.

[deathbybandaid]

updated, and squashed

[deathbybandaid]

I went an added step and added tests

[dgw]

I'm 99% 👍; just want to make these two docstrings consistent with their siblings.

[dgw]

🎉 🙏