Fix bug with pending patch removal

[veruu]

Fix bug with pending patch removal

If the patch moved between Patchwork projects during the time it's being
tested, it gets stuck in the pending patch table. Since patch IDs are
unique per instance (and don't depend on the project), change the
pending patch table to contain Patchwork instance's base URL in addition
to ID of (base URL, patch ID) pair. This solves the problem with patch
removal from pending table, while keeps the original information needed
for proper testing.

Also introduce a way to migrate the database after upgrades since it's
needed for this change.

Fixes #57

Signed-off-by: Veronika Kabatova vkabatov@redhat.com

[coveralls]

Pull Request Test Coverage Report for Build 222

• 5 of 5 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.04%) to 53.574%

---

Totals
Change from base Build 215:	-0.04%
Covered Lines:	682
Relevant Lines:	1273

---

💛 - Coveralls

[spbnick]

This still doesn't update get_last_pending_patch, get_last_pending_patch_date, and get_expired_pending_patches, which would break sktm in various ways. Search for pendingpatches in sktm/db.py to find all the places to update. Please also update the FIXME above the database creation script. Thank you!

[spbnick]

Could we maybe have this (and paragraph above) use ~/.sktm.db so it works as is with the default install, regardless of the current directory (which will probably be db_migrations)?

Not necessary for my ACK.

[spbnick]

Can we have this named pendingpatches_new for clarity?

Not necessary for my ACK.

[spbnick]

We need to add PRIMARY KEY(baseurl, id) next, in a separate PR, to fix the bug mentioned in set_patchset_pending description.

Not necessary for my ACK.

[veruu]

Shouldn't be hard to add here 👍

[spbnick]

This bug.

[veruu]

I must have messed up something during the rebases because I'm pretty sure I fixed those methods yesterday. Ah well, thanks for noticing!

[spbnick]

I dug around the SQL syntax a little and it seems we can simply change the delete statement to only match the baseurl and the patch id like this:

DELETE FROM pendingpatches
       WHERE patchsource_id IN (SELECT DISTINCT id
                                       FROM patchsource
                                       WHERE baseurl = ?) AND
             id = ?

E.g.:

DELETE FROM pendingpatches
       WHERE patchsource_id IN (SELECT DISTINCT id
                                       FROM patchsource
                                       WHERE baseurl = "https://patchwork.kernel.org") AND
             id = 10451123

This way we won't have to update the schema at all, nor have duplication, nor so many code changes.
It will still be a hack against the schema, and we'll still need to fix it later, though.
What do you think?

[veruu]

That should work too. Agreed that we have a lot of DB stuff to fix later, and are only pushing there more and more things that we'll need to eventually fix. But for now, it should be fine.

I'd still keep the migration infrastructure in a separate commit though, we'd need it :)

[spbnick]

Yes, let's keep the migration infrastructure. Thanks :)

[veruu]

@spbnick updated, and added a comment about patchsource table into issue #80

[spbnick]

Looks great, thank you, Veronika :)!