Avoid relying on incremental CF#s (needs CF changes part of PR 62)

[polobo]

logging.info("pulling submissions for current commitfest")
cfbot_commitfest.pull_submissions(conn, commitfest_id)
logging.info("pulling submissions for next commitfest")
cfbot_commitfest.pull_submissions(conn, commitfest_id + 1)

The above seems quite fragile. The CF only will ever have a single Open CF and optionally a single In Progress CF. It is also going to have a single Parked/Draft CF soon. When we add that CF we should do so enforcing these single-instances of each and add an API endpoint that converts the named CFs into their actual numbers. This can then query that API and execute pull_submissions conditionally for each of the CF types.

Adding the API endpoint to pgcommitfest PR 62 seems trivial as would using it here to get rid of the “+1” code. pseudo-code:

open_cf, inprogress_cf, parked_cf = rpc.get_non_closed_cfs(conn)
if open_cf: pull_submission(conn, open_cf)
If inprogress_cf: pull_submission_conn, inprogress_cf)
If parked_cf: pull_submission(conn, parked_cf)

[macdice]

Not only was it fragile but it broke :-).

Yeah, generally making cfbot know a lot less seems like the right way, but I wonder if we can go further: does it need to know about separate commitfests at all? We've been gradually removing those IDs from a few places, branch names, patch URLs, etc etc, and it'd be nice if cfbot just had a single list of submissions... which would imply a single API endpoint for "gimme all the submissions I should be building", and that's all it needs to know about, does that make sense?

Also, with Magnus we agreed in principle that the CF App should start sending HTTP push notifications for submission changes to cfbot, instead of the cfbot pulling them. Maybe there'd still be a CF app RPC endpoint to pull down the whole list that it could use to smooth over lost notifications? Depends what level of reliability we engineer for that notification scheme.

Long story short, sure, happy to make whatever small changes you need but it sounds like we really need a thread over on the discord so we can synchronise our plans for cf/cfbot API stuff in a more flexible discussion format. See you over there!

[polobo]

CFBot maintains a Priority Queue of Submissions. Basically circular.

CFApp pushes Enqueue(PatchID|MessageID), Dequeue(PatchID) events.

Enqueue:
While open, is attached to a CF which is open
New message with patch attachments detected (includes first such message during attach thread)

Dequeue:
Commitfest closes while it is open (single bulk update possible, let CFBot iterate on the set)
Patch is closed

Sync (on-demand or maybe daily as a safety valve):
Dequeue any not present in sync result
Enqueue any present in sync result that has changed (new "current" message containing patches).
If done daily should be a no-op if everything is working correctly and no transient concurrency with in-flight events.

An enqueue includes a MessageID, if an existing queued Submission has the same PatchID but a different MessageID that existing queued Submission is dequeued and the new one takes its place. A dequeue with a PatchID simply removes the corresponding Subscription (if present - it should be...). If the PatchID|MessageID isn't different, an enqueue action does not result in any changes.

Upon enqueue (or during its first processing) download the attachments for the Patch|Message and place them into a local cache.
Bitrot checks pull the content from the local cache.
Upon dequeue, remove any downloaded attachments from the local cache.

When enqueuing, determine where in the circular priority queue to place the Submission. Simple policy, behind all enqueued but not yet processed submissions but in front of the first bitrot check submission.

[polobo]

Incorporate an “is_ignored” flag to keep those Submissions around but not undergoing processing. Do not dequeue because of a closed CF but either send “Disable(PatchID)” events or an equivalent bulk update. The sync process can lookup the current ignored submissions along with the known active ones.