Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

All resources: Use Etag to save API quota #143

Merged
merged 2 commits into from
Sep 4, 2018
Merged

All resources: Use Etag to save API quota #143

merged 2 commits into from
Sep 4, 2018

Conversation

radeksimko
Copy link
Contributor

This is to address one part of https://github.com/terraform-providers/terraform-provider-github/issues/5

100 repositories test

Current implementation

  • creation: 300 requests (POST -> PATCH -> GET)
  • refresh: 100 requests (GET)
  • destruction: 100 + 100 requests (GET -> DELETE)

600 requests in total.

Etag (with patch in this PR)

  • creation: 300 requests consumed from quota
  • refresh: 0 requests consumed from quota
  • destruction: 100 requests consumed from quota

400 requests in total, i.e. 33% reduction in quota consumption.

Acceptance tests

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./github -v  -timeout 120m
=== RUN   TestAccGithubIpRangesDataSource_existing
--- PASS: TestAccGithubIpRangesDataSource_existing (1.32s)
=== RUN   TestAccGithubRepositoriesDataSource_basic
--- PASS: TestAccGithubRepositoriesDataSource_basic (7.39s)
=== RUN   TestAccGithubRepositoriesDataSource_noMatch
--- PASS: TestAccGithubRepositoriesDataSource_noMatch (0.85s)
=== RUN   TestAccGithubRepositoryDataSource_fullName_noMatchReturnsError
--- PASS: TestAccGithubRepositoryDataSource_fullName_noMatchReturnsError (0.16s)
=== RUN   TestAccGithubRepositoryDataSource_name_noMatchReturnsError
--- PASS: TestAccGithubRepositoryDataSource_name_noMatchReturnsError (0.16s)
=== RUN   TestAccGithubRepositoryDataSource_fullName_existing
--- PASS: TestAccGithubRepositoryDataSource_fullName_existing (0.76s)
=== RUN   TestAccGithubRepositoryDataSource_name_existing
--- PASS: TestAccGithubRepositoryDataSource_name_existing (0.78s)
=== RUN   TestAccGithubTeamDataSource_noMatchReturnsError
--- PASS: TestAccGithubTeamDataSource_noMatchReturnsError (0.14s)
=== RUN   TestAccGithubUserDataSource_noMatchReturnsError
--- PASS: TestAccGithubUserDataSource_noMatchReturnsError (0.20s)
=== RUN   TestAccGithubUserDataSource_existing
--- PASS: TestAccGithubUserDataSource_existing (2.59s)
=== RUN   TestMigrateGithubWebhookStateV0toV1
--- PASS: TestMigrateGithubWebhookStateV0toV1 (0.00s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestProvider_insecure
--- PASS: TestProvider_insecure (0.14s)
=== RUN   TestAccGithubBranchProtection_basic
--- FAIL: TestAccGithubBranchProtection_basic (3.51s)
	testing.go:518: Step 0 error: Check failed: Check 3/15 error: diff "restrictions.users": (-got +want)
		 [
		+ "terraformcollaborator",
		 ]
=== RUN   TestAccGithubBranchProtection_teams
--- PASS: TestAccGithubBranchProtection_teams (11.14s)
=== RUN   TestAccGithubBranchProtection_emptyItems
--- PASS: TestAccGithubBranchProtection_emptyItems (4.06s)
=== RUN   TestAccGithubBranchProtection_importBasic
--- FAIL: TestAccGithubBranchProtection_importBasic (3.30s)
	testing.go:518: Step 0 error: After applying this step, the plan was not empty:

		DIFF:

		UPDATE: github_branch_protection.master
		  required_pull_request_reviews.0.dismissal_users.#:         "0" => "1"
		  required_pull_request_reviews.0.dismissal_users.862080235: "" => "terraformcollaborator"
		  restrictions.0.users.#:                                    "0" => "1"
		  restrictions.0.users.862080235:                            "" => "terraformcollaborator"

		STATE:

		github_branch_protection.master:
		  ID = 9pfio:master
		  provider = provider.github
		  branch = master
		  enforce_admins = true
		  etag = W/"b1b78920e662f423d3989e851e9cf4ed"
		  repository = 9pfio
		  required_pull_request_reviews.# = 1
		  required_pull_request_reviews.0.dismiss_stale_reviews = true
		  required_pull_request_reviews.0.dismissal_teams.# = 0
		  required_pull_request_reviews.0.dismissal_users.# = 0
		  required_pull_request_reviews.0.include_admins = false
		  required_pull_request_reviews.0.require_code_owner_reviews = true
		  required_status_checks.# = 1
		  required_status_checks.0.contexts.# = 1
		  required_status_checks.0.contexts.663644092 = github/foo
		  required_status_checks.0.include_admins = false
		  required_status_checks.0.strict = true
		  restrictions.# = 1
		  restrictions.0.teams.# = 0
		  restrictions.0.users.# = 0

		  Dependencies:
		    github_repository.test
		github_repository.test:
		  ID = 9pfio
		  provider = provider.github
		  allow_merge_commit = true
		  allow_rebase_merge = true
		  allow_squash_merge = true
		  archived = false
		  auto_init = true
		  default_branch = master
		  description = Terraform Acceptance Test 9pfio
		  etag = W/"82e7f8a78e74dd1c9fb4fd251cd0534e"
		  full_name = terraformtesting/9pfio
		  git_clone_url = git://github.com/terraformtesting/9pfio.git
		  has_downloads = false
		  has_issues = false
		  has_wiki = false
		  homepage_url =
		  html_url = https://github.com/terraformtesting/9pfio
		  http_clone_url = https://github.com/terraformtesting/9pfio.git
		  name = 9pfio
		  private = false
		  ssh_clone_url = git@github.com:terraformtesting/9pfio.git
		  svn_url = https://github.com/terraformtesting/9pfio
		  topics.# = 0
=== RUN   TestAccGithubIssueLabel_basic
--- PASS: TestAccGithubIssueLabel_basic (5.72s)
=== RUN   TestAccGithubIssueLabel_existingLabel
--- PASS: TestAccGithubIssueLabel_existingLabel (3.86s)
=== RUN   TestAccGithubIssueLabel_importBasic
--- PASS: TestAccGithubIssueLabel_importBasic (3.29s)
=== RUN   TestAccGithubIssueLabel_description
--- PASS: TestAccGithubIssueLabel_description (7.53s)
=== RUN   TestAccGithubMembership_basic
--- PASS: TestAccGithubMembership_basic (1.61s)
=== RUN   TestAccGithubMembership_importBasic
--- PASS: TestAccGithubMembership_importBasic (1.54s)
=== RUN   TestAccGithubOrganizationProject_basic
--- PASS: TestAccGithubOrganizationProject_basic (1.51s)
=== RUN   TestAccGithubOrganizationProject_importBasic
--- PASS: TestAccGithubOrganizationProject_importBasic (1.60s)
=== RUN   TestAccGithubOrganizationWebhook_basic
--- PASS: TestAccGithubOrganizationWebhook_basic (2.02s)
=== RUN   TestAccGithubOrganizationWebhook_secret
--- PASS: TestAccGithubOrganizationWebhook_secret (1.23s)
=== RUN   TestAccGithubRepositoryCollaborator_basic
--- PASS: TestAccGithubRepositoryCollaborator_basic (3.53s)
=== RUN   TestAccGithubRepositoryCollaborator_importBasic
--- PASS: TestAccGithubRepositoryCollaborator_importBasic (4.15s)
=== RUN   TestSuppressDeployKeyDiff
--- PASS: TestSuppressDeployKeyDiff (0.00s)
=== RUN   TestAccGithubRepositoryDeployKey_basic
--- PASS: TestAccGithubRepositoryDeployKey_basic (3.12s)
=== RUN   TestAccGithubRepositoryDeployKey_importBasic
--- PASS: TestAccGithubRepositoryDeployKey_importBasic (3.05s)
=== RUN   TestAccGithubRepositoryProject_basic
--- PASS: TestAccGithubRepositoryProject_basic (3.41s)
=== RUN   TestAccGithubRepositoryProject_importBasic
--- PASS: TestAccGithubRepositoryProject_importBasic (3.35s)
=== RUN   TestAccGithubRepository_basic
--- PASS: TestAccGithubRepository_basic (3.73s)
=== RUN   TestAccGithubRepository_archive
--- PASS: TestAccGithubRepository_archive (2.60s)
=== RUN   TestAccGithubRepository_archiveUpdate
--- PASS: TestAccGithubRepository_archiveUpdate (3.80s)
=== RUN   TestAccGithubRepository_importBasic
--- PASS: TestAccGithubRepository_importBasic (2.44s)
=== RUN   TestAccGithubRepository_defaultBranch
--- PASS: TestAccGithubRepository_defaultBranch (7.52s)
=== RUN   TestAccGithubRepository_templates
--- PASS: TestAccGithubRepository_templates (3.51s)
=== RUN   TestAccGithubRepository_topics
--- PASS: TestAccGithubRepository_topics (5.78s)
=== RUN   TestAccGithubRepository_autoInitForceNew
--- PASS: TestAccGithubRepository_autoInitForceNew (5.95s)
=== RUN   TestAccGithubRepositoryWebhook_basic
--- PASS: TestAccGithubRepositoryWebhook_basic (5.22s)
=== RUN   TestAccGithubRepositoryWebhook_secret
--- PASS: TestAccGithubRepositoryWebhook_secret (3.51s)
=== RUN   TestAccGithubRepositoryWebhook_importBasic
--- PASS: TestAccGithubRepositoryWebhook_importBasic (3.36s)
=== RUN   TestAccGithubRepositoryWebhook_importSecret
--- PASS: TestAccGithubRepositoryWebhook_importSecret (4.07s)
=== RUN   TestAccGithubTeamMembership_basic
--- PASS: TestAccGithubTeamMembership_basic (4.48s)
=== RUN   TestAccGithubTeamMembership_importBasic
--- PASS: TestAccGithubTeamMembership_importBasic (2.44s)
=== RUN   TestAccGithubTeamRepository_basic
--- PASS: TestAccGithubTeamRepository_basic (4.79s)
=== RUN   TestAccGithubTeamRepository_importBasic
--- PASS: TestAccGithubTeamRepository_importBasic (3.65s)
=== RUN   TestAccCheckGetPermissions
--- PASS: TestAccCheckGetPermissions (0.00s)
=== RUN   TestAccGithubTeam_basic
--- PASS: TestAccGithubTeam_basic (2.81s)
=== RUN   TestAccGithubTeam_slug
--- PASS: TestAccGithubTeam_slug (1.41s)
=== RUN   TestAccGithubTeam_hierarchical
--- PASS: TestAccGithubTeam_hierarchical (2.85s)
=== RUN   TestAccGithubTeam_importBasic
--- PASS: TestAccGithubTeam_importBasic (1.45s)
=== RUN   TestAccGithubUserGpgKey_basic
--- PASS: TestAccGithubUserGpgKey_basic (2.00s)
=== RUN   TestAccGithubUserSshKey_basic
--- PASS: TestAccGithubUserSshKey_basic (1.33s)
=== RUN   TestAccGithubUserSshKey_importBasic
--- PASS: TestAccGithubUserSshKey_importBasic (1.33s)
=== RUN   TestEtagTransport
--- PASS: TestEtagTransport (0.00s)
=== RUN   TestAccGithubUtilRole_validation
--- PASS: TestAccGithubUtilRole_validation (0.00s)
=== RUN   TestAccGithubUtilTwoPartID
--- PASS: TestAccGithubUtilTwoPartID (0.00s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-github/github	167.029s

Test failures are result of eventually consistent GitHub API.

@radeksimko radeksimko added the Type: Feature New feature or request label Sep 4, 2018
@ghost ghost added the size/XL label Sep 4, 2018
bflad
bflad approved these changes Sep 4, 2018
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! One minor nitpick otherwise LGTM. 🚀

github/transport_test.go Outdated
}))
}

type response struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpick: Naming here should probably be mockResponse or testResponse

@ghost ghost added the size/XL label Sep 4, 2018
@radeksimko radeksimko merged commit 6ed5159 into master Sep 4, 2018
@radeksimko radeksimko deleted the f-rate-limits branch September 4, 2018 12:43
@radeksimko radeksimko mentioned this pull request Sep 5, 2018
Merged
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
@radeksimko
Merge pull request integrations#143 from terraform-providers/f-rate-l…
9dd585a 
…imits

All resources: Use Etag to save API quota
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bflad bflad bflad approved these changes

Assignees
No one assigned
Labels
size/XL Type: Feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@radeksimko @bflad