fix(branch-protection): Use more efficient function to fetch actors #1331
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 05a8875.
branchProtectionResourceData is very slow to call so used branchProtectionResourceDataActors instead
Closed
kfcampbell
approved these changes
Oct 27, 2022
There was a problem hiding this comment.
Input on #1328 about the efficiency of these changes is appreciated! Thanks for the test data, @dion-gionet.
kazaker
pushed a commit
to auto1-oss/terraform-provider-github
that referenced
this pull request
Dec 28, 2022
…ntegrations#1331) * feat(branch-protection): Push/Reviewer actors can be specified by name This reverts commit 05a8875. * fix(branch-protection): Use more efficient function to fetch actors branchProtectionResourceData is very slow to call so used branchProtectionResourceDataActors instead
avidspartan1
pushed a commit
to avidspartan1/terraform-provider-github
that referenced
this pull request
Feb 5, 2024
…ntegrations#1331) * feat(branch-protection): Push/Reviewer actors can be specified by name This reverts commit d825afa. * fix(branch-protection): Use more efficient function to fetch actors branchProtectionResourceData is very slow to call so used branchProtectionResourceDataActors instead
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1328
Brings back #1020 but fixed the slowness that was caused by slow function calls (branchProtectionResourceData).
I've tested multiple scenarios from provider versions 5.4.0, 5.5.0 and my patch to make sure the performance regression is gone or at least very comparable to the speed of 5.5.0.
This was tested on a test org with over 160 repos (1 github_team_repository, 1 github_branch_default and 1 github_branch_protection resource per repo)
As you can see from the results below, the speed has greatly improved by using a trimmed down version of branchProtectionResourceData. You can see exactly what I changed on commit f308e30
Test results
Plan using node IDs
Planning on 5.4.0
Planning with my patch
Planning on 5.5.0
Plan using user login name
Planning on 5.4.0
Planning with my patch
Apply change on github_branch_protection for all resources using node IDs and user login names
Applying on 5.5.0 with node IDs
Applying with my patch with user login names. This is slower than applying with node IDs since it has to fetch the ID before applying
Applying with my patch with node IDs
cc @kfcampbell