GitHub Enterprise feature parity

[jianyuan]

This Pull Request enables GitHub Enterprise integration feature parity with the GitHub integration.

[codecov]

Codecov Report

Merging #52951 (89aa66a) into master (df24d61) will increase coverage by 0.46%.
Report is 80 commits behind head on master.
The diff coverage is 67.50%.

❗ Current head 89aa66a differs from pull request most recent head 46b4cec. Consider uploading reports for the commit 46b4cec to get more accurate results

@@            Coverage Diff             @@
##           master   #52951      +/-   ##
==========================================
+ Coverage   78.57%   79.03%   +0.46%     
==========================================
  Files        5135     5135              
  Lines      223389   223425      +36     
  Branches    37614    37621       +7     
==========================================
+ Hits       175522   176589    +1067     
+ Misses      42166    41182     -984     
+ Partials     5701     5654      -47     

Files	Coverage Δ
src/sentry/api/bases/external_actor.py	91.39% <ø> (ø)
src/sentry/api/validators/project_codeowners.py	100.00% <ø> (ø)
src/sentry/models/integrations/external_actor.py	93.61% <ø> (ø)
src/sentry/types/integrations.py	91.66% <100.00%> (+0.36%)	⬆️
static/app/views/replays/detail/tagPanel/index.tsx	100.00% <ø> (ø)
...rc/sentry/integrations/github_enterprise/client.py	95.23% <85.71%> (-4.77%)	⬇️
...ntry/integrations/github_enterprise/integration.py	79.75% <61.29%> (-4.69%)	⬇️

... and 113 files with indirect coverage changes

[AniketDas-Tekky]

LGTM on the whole

[leedongwei]

GitHubIntegration extends from CommitContextMixin too. Github Enterprise should have commit metadata too.

[leeandher]

This is true, also looking at some of the procedure for CODEOWNERS, it looks like GitHubEnterpriseIntegration will also need the codeowners_locations set for it to work.

Likely also makes sense to set repo_search to True

[leeandher]

There may also be quite a few errors that only appear when you attempt to actually use stack trace linking/codeowners with github enterprise, since tracing these code paths is a bit confusing. Could you upload a video/screenshot of the features working with a GHE instance?

For example, get_commit_context is needed for stack trace linking, or get_trees_for_org for deriving codemappings. I'd say to get feature parity, its ideal to either inherit the entire GithubIntegration class, or create a module where the functions can be shared across both if there's a lot of overlap.

I predict that if we tried to use these features as is, we'd hit some NotImplementedErrors that might lack test coverage.

[leeandher]

minor correction to my above comment* The get_commit_context and CommitContextMixin are for the suspect commits feature, not stack trace linking.

[Dhrumil-Sentry]

@leeandher When you review this PR- I think the text in the stack trace link should be the same as we have GitHub 🤔 - I don't think we need to specify GitHub Enterprise in the text

[Dhrumil-Sentry]

@leeandher In the video @jianyuan recorded for this- Even suspect commits showed up for the issue- It would be great to know if this was the Git Blame suspect commit or the suspect commits based on associated commits. Will help with documentation and questions from SEs

[leeandher]

Looks great! Thanks for adding the tests! There are a few merge conflicts to resolve, but after that I'll trigger CI and get this merged.

@Dhrumil-Sentry per your comments:

• For the purposes of this PR I think we can leave the frontend with "Github Enterprise" since we want frontend/backend changes isolated. I also think I'm in favour of leaving it as GHE in case they have both GitHub and Github Enterprise integrations installed. Not common, but I think we always leave 'enterprise' in the title for most pages that distinguish the two (like linking/creating issues).
  

• As far as I can tell the suspect committers are still coming from associations with commits and releases. I don't believe we use blames yet, but this may be a better question for the Issues teams, since I don't have much experience with that area

[jianyuan]

@leeandher I have fixed the merge conflicts.

[leeandher]

/gcbrun

[leeandher]

/gcbrun

As per review comments, the changes have been addressed