Fix: Eliminate redundant full table scans in messages and events collection (phase 1, only events.py)

[PredictiveManish]

Description

augur/tasks/github/events.py

• Built issue_url_to_id_map and pr_url_to_id_map once in BulkGithubEventCollection.collect() before the batch loop

• Updated _process_events(), _process_issue_events(), and _process_pr_events() to accept mappings as parameters

• Removed redundant _get_map_from_*() calls from batch processing methods
  Notes for Reviewers

• This PR fixes (partially) Full table scans on every batch in messages and events collection #3440

Signed commits

• Yes, I signed my commits.

[MoralCode]

code LGTM! Would like to better understand how this has been tested/whether it still needs testing to confirm it works

[PredictiveManish]

code LGTM! Would like to better understand how this has been tested/whether it still needs testing to confirm it works

Actually yes it needs testing to confirm! Will confirm when it's done, facing a little issue in setting up as I have purchased a new device so will share when tested.

[MoralCode]

marked waiting due to waiting for testing (also probably needs a rebase)

[shlokgilda]

LGTM. Waiting to hear back about the testing status.

[MoralCode]

Can you resolve the merge conflicts?

[PredictiveManish]

Can you resolve the merge conflicts?

Yes, sure!

[shlokgilda]

LGTM.

[MoralCode]

Once this has been tested, it can be merged.

[sgoggins]

LGTM

[shlokgilda]

The optimization idea is right but issue_url_to_id_map and pr_url_to_id_map are never actually defined in collect(). They're just passed as undefined variables to _process_events. This will crash.

You need to add these two lines in collect() after repo_id is set, before the loop:

issue_url_to_id_map = self._get_map_from_issue_url_to_id(repo_id)     
pr_url_to_id_map = self._get_map_from_pr_url_to_id(repo_id)

The helper methods still exist but nothing calls them now, so they're dead code until this is wired up.

[shlokgilda]

Left an inline comment. Should be GTG after that

[shlokgilda]

This call wasn't updated to pass the new map arguments. It'll raise a TypeError at runtime for any repo with more events than event_batch_size. Needs to be:

self._process_events(events, repo_id, issue_url_to_id_map, pr_url_to_id_map)

Also the comment right above (# making this a decent size since process_events retrieves all the issues and prs each time) is now stale and should be removed.