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.

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

[Internal] Retry on 504 when calling the permission API #4355

Merged
merged 2 commits into from
Dec 27, 2024

Conversation

renaudhartert-db
Copy link
Contributor

Changes

This PR adds logic to retry failed get calls to the permissions API when the error is a 504. This solution is meant to be temporary and will be removed as soon as such retries are handled natively in the Databricks Go SDK.

Tests

Complete coverage of the added retrier.

Copy link
Contributor

@rauchy rauchy left a comment

Choose a reason for hiding this comment

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

LGTM. Do we want to stub out a 504 in resource_permissions_test.go and verify the retry is kicking in there?

Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 4355
  • Commit SHA: 1cf38c5c8e2d1ea191e43989144bd38aa4d51323

Checks will be approved automatically on success.

@eng-dev-ecosystem-bot
Copy link
Collaborator

Test Details: go/deco-tests/12518392447

@renaudhartert-db
Copy link
Contributor Author

LGTM. Do we want to stub out a 504 in resource_permissions_test.go and verify the retry is kicking in there?

Agreed, done.

@renaudhartert-db renaudhartert-db marked this pull request as ready for review December 27, 2024 16:45
@renaudhartert-db renaudhartert-db requested review from a team as code owners December 27, 2024 16:45
@renaudhartert-db renaudhartert-db requested review from parthban-db and removed request for a team December 27, 2024 16:45
@renaudhartert-db renaudhartert-db added this pull request to the merge queue Dec 27, 2024
Merged via the queue into main with commit 1e5443d Dec 27, 2024
12 checks passed
@renaudhartert-db renaudhartert-db deleted the renaud.hartert/retry504 branch December 27, 2024 17:51
renaudhartert-db added a commit that referenced this pull request Dec 30, 2024
### Bug Fixes

 * Reflect backend updates in state for databricks_app ([#4337](#4337)).

### Documentation

 * Update `databricks_workspace_conf` documentation ([#4334](#4334)).
 * apply `make fmt-docs` to all docs ([#4344](#4344)).

### Internal Changes

 * Generate both SdkV2-compatible and Plugin Framework-compatible structures ([#4332](#4332)).
 * Mark TestAccServicePrincipalResourceOnAzure test as flaky ([#4333](#4333)).
 * Retry on 504 when calling the permission API ([#4355](#4355)).
renaudhartert-db added a commit that referenced this pull request Dec 30, 2024
### Bug Fixes

 * Reflect backend updates in state for databricks_app ([#4337](#4337)).

### Documentation

 * Update `databricks_workspace_conf` documentation ([#4334](#4334)).
 * apply `make fmt-docs` to all docs ([#4344](#4344)).

### Internal Changes

 * Generate both SdkV2-compatible and Plugin Framework-compatible structures ([#4332](#4332)).
 * Mark TestAccServicePrincipalResourceOnAzure test as flaky ([#4333](#4333)).
 * Retry on 504 when calling the permission API ([#4355](#4355)).
github-merge-queue bot pushed a commit that referenced this pull request Dec 30, 2024
### Bug Fixes

* Reflect backend updates in state for databricks_app
([#4337](#4337)).


### Documentation

* Update `databricks_workspace_conf` documentation
([#4334](#4334)).
* apply `make fmt-docs` to all docs
([#4344](#4344)).


### Internal Changes

* Generate both SdkV2-compatible and Plugin Framework-compatible
structures
([#4332](#4332)).
* Mark TestAccServicePrincipalResourceOnAzure test as flaky
([#4333](#4333)).
* Retry on 504 when calling the permission API
([#4355](#4355)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants