Refactor pendingpatches #111
Conversation
Allow sktm to store the name and build ID of the Jenkins job that is spawned to run tests. Signed-off-by: Major Hayden <[email protected]>
Add patches to the `patch` table as soon as they are received. Alter the `pendingpatches` table to link test jobs (`pendingjobs`) with patches in the `patch` table. Store each test job in the `pendingjobs` table and refer to the table when checking on tested patches. Signed-off-by: Major Hayden <[email protected]>
Signed-off-by: Major Hayden <[email protected]>
major
force-pushed
the
patch-table-source-of-truth
branch
from
5387015
to
70b3535
Compare
Pull Request Test Coverage Report for Build 351
💛 - Coveralls |
| @@ -0,0 +1,8 @@ | |||
| -- NOTE: Apply this change only when no patches are pending. | |||
There was a problem hiding this comment.
I get that this is due to not knowing how to link the pending jobs with patches in the table, but there's a chance people applying the migration wouldn't open and read the file itself since they don't need to. Can you make a subsection in the README (under Database upgrading) for migration limitations, and mention this condition there?
| for patch_url in patch_urls] | ||
|
|
||
| # Add the patches to the database | ||
| self.db.commit_series(patches) |
There was a problem hiding this comment.
The patches should be included in the patch table only after they completed the testing. I understand that you want to save every grabbed patch into the table, but the table was meant to say "this is what was fully processed" and now we won't have that. Granted, the question is whether we do need that functionality, but that belongs more to the discussion around #80.
| [(patch_id, patch_date, sourceid, tstamp) for | ||
| (patch_id, patch_date) in series_data]) | ||
| self.cur.executemany( | ||
| 'INSERT OR REPLACE INTO pendingpatches ' |
There was a problem hiding this comment.
Given that we can have same patch IDs across different Patchworks, we might end up overwriting entries we shouldn't. We are lucky we haven't run into this issue yet, but since you are touching the same logic here and carrying the bug over, can you fix the bug as well?
| for series in series_dropped: | ||
| logging.info("dropped series: %s", series.get_obj_url_list()) | ||
|
|
||
| # Retrieve all data and save dropped patches in the DB |
There was a problem hiding this comment.
|
@veruu What if we did something like this:
This would reduce the complexity in the database and let the |
|
The downside would be that we would have the Jenkins data appearing repeatedly in the |
This PR adjusts
sktmto store patches in the database as soon as they are received. It also transforms thependingpatchestable into a linkage between a list of patches and a Jenkins job that is testing those patches.This work is required so that sktm can exit after queueing jobs and check on those jobs later as described in #110.