Added retries for getting refreshable token by tanmay-db · Pull Request #500 · databricks/databricks-sdk-go

tanmay-db · 2023-06-14T11:54:58Z

nfx

please add tests. e.g. create a mocked token source - e.g. a function type, that has a Token() (*oauth2.Token, error) method and test for different retriable and non-retriable scenarios

config/oauth_visitors.go

tanmay-db · 2023-06-14T15:23:12Z

config/oauth_visitors.go

+		if err == nil {
+			return token, nil
+		}
+		var retryKeywords = []string{"throttled", "too many requests", "429", "request limit exceeded", "rate limit"}


@nfx Do you have some other keywords that we should also check against?

this is good enough for now:)

tanmay-db · 2023-06-14T15:23:54Z

config/oauth_visitors_test.go

+type mockTokenSource struct {
+	mockedTokenFunc func() (*oauth2.Token, error)
+}
+
+func (m mockTokenSource) Token() (*oauth2.Token, error) {
+	return m.mockedTokenFunc()
+}


It should be outside the tests as it's being used by three tests, putting this inside will increase code duplication.

it's fine to do it this way.

codecov-commenter · 2023-06-14T15:24:13Z

Codecov Report

Patch coverage: 81.81% and project coverage change: -0.03 ⚠️

Comparison is base (e8225d5) 18.92% compared to head (0e4f319) 18.89%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #500      +/-   ##
==========================================
- Coverage   18.92%   18.89%   -0.03%     
==========================================
  Files          82       83       +1     
  Lines        8988     9011      +23     
==========================================
+ Hits         1701     1703       +2     
- Misses       7139     7160      +21     
  Partials      148      148

Impacted Files	Coverage Δ
config/oauth_visitors.go	`48.14% <81.81%> (+21.83%)`	⬆️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

nfx

lgtm

nfx · 2023-06-14T15:27:09Z

config/oauth_visitors_test.go

+type mockTokenSource struct {
+	mockedTokenFunc func() (*oauth2.Token, error)
+}
+
+func (m mockTokenSource) Token() (*oauth2.Token, error) {
+	return m.mockedTokenFunc()
+}


it's fine to do it this way.

# Version changelog ## 0.10.0 * Added log level support to SimpleLogger. Default logger now prints only `INFO` level messages. To replicate more verbose behavior from the previous versions, set the `DEBUG` level in `SimpleLogger` explicitly ([#426](#426)). * Added `Upload` and `Download` methods where applicable ([#423](#423)). * Added auth tests for a databrickscfg file with an empty DEFAULT profile ([#496](#496)). * Added more integration tests and examples ([#503](#503)). * Added preview level functions for services and methods to OpenAPI code ([#497](#497)). * Added IsAllRequiredFieldsPrimitive helper function ([#404](#404)). * Added isCrudCreate method ([#416](#416)). * Added retries for getting refreshable token ([#500](#500)). * Added type update when entity fields are updated ([#411](#411)). * Don't panic but return error from NewAccountClient ([#422](#422)). * Error when opening a DBFS directory for reading ([#415](#415)). * Fixed cleanup logic in SQL warehouse integration test ([#400](#400)). * Fixed error handling in cluster utility function ([#399](#399)). * Fixed example in README.md ([#494](#494)). * Fixed nondeterminism in workspace filesystem integration test ([#401](#401)). * Improve command execution interface ([#410](#410)). * Introduce waiters as top-level methods ([#408](#408)). * Regenerate examples if configured ([#518](#518)). * Respect limit field passed in ListRequests ([#407](#407)). * Updated from OpenAPI spec ([#412](#412), [#413](#413), [#421](#421), [#519](#519), [#424](#424)). * Updated API test template with page tokens ([#417](#417)). * Use constants instead of hardcoding strings ([#402](#402)). * Use `x-databricks-is-accounts` flag to determine whether a service is an account level service ([#420](#420)). API Changes: github.com/databricks/databricks-sdk-go/service/serving - (*ServingEndpointsAPI).Create: changed from func(context.Context, CreateServingEndpoint) (*ServingEndpointDetailed, error) to func(context.Context, CreateServingEndpoint) (*WaitGetServingEndpointNotUpdating[ServingEndpointDetailed], error) - (*ServingEndpointsAPI).List: removed - (*ServingEndpointsAPI).UpdateConfig: changed from func(context.Context, EndpointCoreConfigInput) (*ServingEndpointDetailed, error) to func(context.Context, EndpointCoreConfigInput) (*WaitGetServingEndpointNotUpdating[ServingEndpointDetailed], error) Compatible changes: - (*ServingEndpointsAPI).ListAll: added - (*ServingEndpointsAPI).WaitGetServingEndpointNotUpdating: added - WaitGetServingEndpointNotUpdating: added github.com/databricks/databricks-sdk-go - WorkspaceClient.CommandExecutor: removed - WorkspaceClient.CommandExecution: added - WorkspaceClient.Connections: added - WorkspaceClient.Files: added - WorkspaceClient.SystemSchemas: added github.com/databricks/databricks-sdk-go/service/sql - (*WarehousesAPI).Create: changed from func(context.Context, CreateWarehouseRequest) (*CreateWarehouseResponse, error) to func(context.Context, CreateWarehouseRequest) (*WaitGetWarehouseRunning[CreateWarehouseResponse], error) - (*WarehousesAPI).DeleteAndWait: removed - (*WarehousesAPI).DeleteByIdAndWait: removed - (*WarehousesAPI).Edit: changed from func(context.Context, EditWarehouseRequest) error to func(context.Context, EditWarehouseRequest) (*WaitGetWarehouseRunning[any], error) - (*WarehousesAPI).GetAndWait: removed - (*WarehousesAPI).GetByIdAndWait: removed - (*WarehousesAPI).Start: changed from func(context.Context, StartRequest) error to func(context.Context, StartRequest) (*WaitGetWarehouseRunning[any], error) - (*WarehousesAPI).Stop: changed from func(context.Context, StopRequest) error to func(context.Context, StopRequest) (*WaitGetWarehouseStopped[any], error) - (*StatementExecutionAPI).ExecuteAndWait: added - (*WarehousesAPI).WaitGetWarehouseRunning: added - (*WarehousesAPI).WaitGetWarehouseStopped: added - FormatCsv: added - WaitGetWarehouseRunning: added - WaitGetWarehouseStopped: added github.com/databricks/databricks-sdk-go/service/jobs - (*JobsAPI).CancelRun: changed from func(context.Context, CancelRun) error to func(context.Context, CancelRun) (*WaitGetRunJobTerminatedOrSkipped[any], error) - (*JobsAPI).GetRunAndWait: removed - (*JobsAPI).RepairRun: changed from func(context.Context, RepairRun) (*RepairRunResponse, error) to func(context.Context, RepairRun) (*WaitGetRunJobTerminatedOrSkipped[RepairRunResponse], error) - (*JobsAPI).RunNow: changed from func(context.Context, RunNow) (*RunNowResponse, error) to func(context.Context, RunNow) (*WaitGetRunJobTerminatedOrSkipped[RunNowResponse], error) - (*JobsAPI).Submit: changed from func(context.Context, SubmitRun) (*SubmitRunResponse, error) to func(context.Context, SubmitRun) (*WaitGetRunJobTerminatedOrSkipped[SubmitRunResponse], error) - (*JobsAPI).WaitGetRunJobTerminatedOrSkipped: added - CreateJob.RunAs: added - JobRunAs: added - JobSettings.RunAs: added - WaitGetRunJobTerminatedOrSkipped: added github.com/databricks/databricks-sdk-go/service/compute - (*ClustersAPI).Create: changed from func(context.Context, CreateCluster) (*CreateClusterResponse, error) to func(context.Context, CreateCluster) (*WaitGetClusterRunning[CreateClusterResponse], error) - (*ClustersAPI).Delete: changed from func(context.Context, DeleteCluster) error to func(context.Context, DeleteCluster) (*WaitGetClusterTerminated[any], error) - (*ClustersAPI).Edit: changed from func(context.Context, EditCluster) error to func(context.Context, EditCluster) (*WaitGetClusterRunning[any], error) - (*ClustersAPI).GetAndWait: removed - (*ClustersAPI).GetByClusterIdAndWait: removed - (*ClustersAPI).Resize: changed from func(context.Context, ResizeCluster) error to func(context.Context, ResizeCluster) (*WaitGetClusterRunning[any], error) - (*ClustersAPI).Restart: changed from func(context.Context, RestartCluster) error to func(context.Context, RestartCluster) (*WaitGetClusterRunning[any], error) - (*ClustersAPI).Start: changed from func(context.Context, StartCluster) error to func(context.Context, StartCluster) (*WaitGetClusterRunning[any], error) - (*CommandExecutionAPI).Cancel: changed from func(context.Context, CancelCommand) error to func(context.Context, CancelCommand) (*WaitCommandStatusCommandExecutionCancelled[any], error) - (*CommandExecutionAPI).Create: changed from func(context.Context, CreateContext) (*Created, error) to func(context.Context, CreateContext) (*WaitContextStatusCommandExecutionRunning[Created], error) - (*CommandExecutionAPI).Execute: changed from func(context.Context, Command) (*Created, error) to func(context.Context, Command) (*WaitCommandStatusCommandExecutionFinishedOrError[Created], error) - (*ClustersAPI).WaitGetClusterRunning: added - (*ClustersAPI).WaitGetClusterTerminated: added - (*CommandExecutionAPI).Start: added - (*CommandExecutionAPI).WaitCommandStatusCommandExecutionCancelled: added - (*CommandExecutionAPI).WaitCommandStatusCommandExecutionFinishedOrError: added - (*CommandExecutionAPI).WaitContextStatusCommandExecutionRunning: added - BaseClusterInfo.DataSecurityMode: added - BaseClusterInfo.DockerImage: added - BaseClusterInfo.SingleUserName: added - ClusterAttributes.DataSecurityMode: added - ClusterAttributes.DockerImage: added - ClusterAttributes.SingleUserName: added - ClusterInfo.DockerImage: added - CommandExecutorV2: added - EditCluster.DataSecurityMode: added - EditCluster.DockerImage: added - EditCluster.SingleUserName: added - WaitCommandStatusCommandExecutionCancelled: added - WaitCommandStatusCommandExecutionFinishedOrError: added - WaitContextStatusCommandExecutionRunning: added - WaitGetClusterRunning: added - WaitGetClusterTerminated: added github.com/databricks/databricks-sdk-go/service/pipelines - (*PipelinesAPI).GetAndWait: removed - (*PipelinesAPI).GetByPipelineIdAndWait: removed - (*PipelinesAPI).Reset: changed from func(context.Context, ResetRequest) error to func(context.Context, ResetRequest) (*WaitGetPipelineRunning[any], error) - (*PipelinesAPI).Stop: changed from func(context.Context, StopRequest) error to func(context.Context, StopRequest) (*WaitGetPipelineIdle[any], error) - (*PipelinesAPI).WaitGetPipelineIdle: added - (*PipelinesAPI).WaitGetPipelineRunning: added - WaitGetPipelineIdle: added - WaitGetPipelineRunning: added github.com/databricks/databricks-sdk-go/service/sharing - (*ProvidersAPI).ListShares: removed - (*ProvidersAPI).ListSharesAll: added github.com/databricks/databricks-sdk-go/service/workspace - ExportFormatAuto: removed - ExportRequest.DirectDownload: removed - Import.Format: changed from ExportFormat to ImportFormat - (*WorkspaceAPI).Download: added - (*WorkspaceAPI).ReadFile: added - (*WorkspaceAPI).Upload: added - (*WorkspaceAPI).WriteFile: added - DownloadFormat: added - DownloadOption: added - ImportFormat: added - ImportFormatAuto: added - ImportFormatDbc: added - ImportFormatHtml: added - ImportFormatJupyter: added - ImportFormatRMarkdown: added - ImportFormatSource: added - UploadFormat: added - UploadLanguage: added - UploadOption: added - UploadOverwrite: added github.com/databricks/databricks-sdk-go/service/catalog - (*FunctionsAPI).List: removed - (*MetastoresAPI).Assign: changed from func(context.Context, CreateMetastoreAssignment) error to func(context.Context, CreateMetastoreAssignment) error - (*MetastoresAPI).Create: changed from func(context.Context, CreateMetastore) (*MetastoreInfo, error) to func(context.Context, CreateMetastore) (*MetastoreInfo, error) - (*MetastoresAPI).Update: changed from func(context.Context, UpdateMetastore) (*MetastoreInfo, error) to func(context.Context, UpdateMetastore) (*MetastoreInfo, error) - (*MetastoresAPI).UpdateAssignment: changed from func(context.Context, UpdateMetastoreAssignment) error to func(context.Context, UpdateMetastoreAssignment) error - (*StorageCredentialsAPI).Create: changed from func(context.Context, CreateStorageCredential) (*StorageCredentialInfo, error) to func(context.Context, CreateStorageCredential) (*StorageCredentialInfo, error) - (*StorageCredentialsAPI).Update: changed from func(context.Context, UpdateStorageCredential) (*StorageCredentialInfo, error) to func(context.Context, UpdateStorageCredential) (*StorageCredentialInfo, error) - (*TablesAPI).ListSummaries: removed - CreateMetastore.Name: removed - CreateMetastore.Region: removed - CreateMetastore.StorageRoot: removed - CreateMetastore: changed from CreateMetastore to CreateMetastore - CreateMetastoreAssignment.DefaultCatalogName: removed - CreateMetastoreAssignment: changed from CreateMetastoreAssignment to CreateMetastoreAssignment - CreateStorageCredential.AwsIamRole: removed - CreateStorageCredential.AzureServicePrincipal: removed - CreateStorageCredential.Comment: removed - CreateStorageCredential.GcpServiceAccountKey: removed - CreateStorageCredential.Name: removed - CreateStorageCredential.ReadOnly: removed - CreateStorageCredential.SkipValidation: removed - CreateStorageCredential: changed from CreateStorageCredential to CreateStorageCredential - GcpServiceAccountKey: removed - ListFunctionsResponse.Schemas: removed - MetastoreAssignment.WorkspaceId: changed from string to int64 - StorageCredentialInfo.GcpServiceAccountKey: removed - UpdateMetastore.DeltaSharingOrganizationName: removed - UpdateMetastore.DeltaSharingRecipientTokenLifetimeInSeconds: removed - UpdateMetastore.DeltaSharingScope: removed - UpdateMetastore.Id: removed - UpdateMetastore.Name: removed - UpdateMetastore.Owner: removed - UpdateMetastore.PrivilegeModelVersion: removed - UpdateMetastore.StorageRootCredentialId: removed - UpdateMetastore: changed from UpdateMetastore to UpdateMetastore - UpdateMetastoreAssignment.DefaultCatalogName: removed - UpdateMetastoreAssignment: changed from UpdateMetastoreAssignment to UpdateMetastoreAssignment - UpdateStorageCredential.AwsIamRole: removed - UpdateStorageCredential.AzureServicePrincipal: removed - UpdateStorageCredential.Comment: removed - UpdateStorageCredential.Force: removed - UpdateStorageCredential.GcpServiceAccountKey: removed - UpdateStorageCredential.Owner: removed - UpdateStorageCredential.ReadOnly: removed - UpdateStorageCredential.SkipValidation: removed - UpdateStorageCredential: changed from UpdateStorageCredential to UpdateStorageCredential - ValidateStorageCredential.GcpServiceAccountKey: removed - (*FunctionsAPI).FunctionInfoNameToFullNameMap: added - (*FunctionsAPI).ListAll: added - (*TablesAPI).ListSummariesAll: added - AccountsCreateMetastore: added - AccountsCreateMetastoreAssignment: added - AccountsCreateStorageCredential: added - AccountsUpdateMetastore: added - AccountsUpdateMetastoreAssignment: added - AccountsUpdateStorageCredential: added - AzureManagedIdentity: added - ConnectionInfo: added - ConnectionType: added - ConnectionTypeDatabricks: added - ConnectionTypeMysql: added - ConnectionTypePostgresql: added - ConnectionTypeRedshift: added - ConnectionTypeSnowflake: added - ConnectionTypeSqldw: added - ConnectionTypeSqlserver: added - ConnectionsAPI: added - ConnectionsService: added - CreateConnection: added - CreateMetastore.MetastoreInfo: added - CreateMetastoreAssignment.MetastoreAssignment: added - CreateStorageCredential.CredentialInfo: added - CredentialType: added - CredentialTypeUsernamePassword: added - DatabricksGcpServiceAccountResponse: added - DeleteConnectionRequest: added - DisableRequest: added - GetConnectionRequest: added - ListConnectionsResponse: added - ListFunctionsResponse.Functions: added - ListSystemSchemasRequest: added - ListSystemSchemasResponse: added - NewConnections: added - NewSystemSchemas: added - OptionsKvPairs: added - PropertiesKvPairs: added - StorageCredentialInfo.AzureManagedIdentity: added - StorageCredentialInfo.DatabricksGcpServiceAccount: added - SystemSchemaInfo: added - SystemSchemaInfoState: added - SystemSchemaInfoStateDisableinitialized: added - SystemSchemaInfoStateEnablecompleted: added - SystemSchemaInfoStateEnableinitialized: added - SystemSchemaInfoStateUnavailable: added - SystemSchemasAPI: added - SystemSchemasService: added - UpdateConnection: added - UpdateMetastore.MetastoreInfo: added - UpdateMetastoreAssignment.MetastoreAssignment: added - UpdateStorageCredential.CredentialInfo: added - ValidateStorageCredential.AzureManagedIdentity: added - ValidateStorageCredential.DatabricksGcpServiceAccount: added github.com/databricks/databricks-sdk-go/service/ml - GetModelResponse.RegisteredModel: removed - GetModelResponse.RegisteredModelDatabricks: added github.com/databricks/databricks-sdk-go/service/files - FilesAPI: added - FilesService: added - NewFiles: added github.com/databricks/databricks-sdk-go/logger - SimpleLogger.Level: added github.com/databricks/databricks-sdk-go/service/provisioning - (*WorkspacesAPI).Create: changed from func(context.Context, CreateWorkspaceRequest) (*Workspace, error) to func(context.Context, CreateWorkspaceRequest) (*WaitGetWorkspaceRunning[Workspace], error) - (*WorkspacesAPI).Update: changed from func(context.Context, UpdateWorkspaceRequest) error to func(context.Context, UpdateWorkspaceRequest) (*WaitGetWorkspaceRunning[any], error) - (*WorkspacesAPI).WaitGetWorkspaceRunning: added - WaitGetWorkspaceRunning: added github.com/databricks/databricks-sdk-go/service/iam - AccountAccessControlAPI: added - AccountAccessControlProxyAPI: added - AccountAccessControlProxyService: added - AccountAccessControlService: added - GetAssignableRolesForResourceRequest: added - GetAssignableRolesForResourceResponse: added - GetRuleSetRequest: added - GrantRule: added - NewAccountAccessControl: added - NewAccountAccessControlProxy: added - RuleSetResponse: added - RuleSetUpdateRequest: added - UpdateRuleSetRequest: added Dependency updates: * Bump github.com/stretchr/testify from 1.8.3 to 1.8.4 ([#406](#406)). * Bump golang.org/x/mod from 0.10.0 to 0.11.0 ([#515](#515)). * Bump golang.org/x/oauth2 from 0.8.0 to 0.9.0 ([#498](#498)). * Bump google.golang.org/api from 0.123.0 to 0.127.0 ([#405](#405), [#425](#425), [#429](#429)).

tanmay-db added 3 commits June 13, 2023 18:33

refreshableRetry

c072a43

upd

0c493ba

upd

e5777e4

tanmay-db changed the title ~~Added retried for getting refreshable token~~ Added retries for getting refreshable token Jun 14, 2023

nfx suggested changes Jun 14, 2023

View reviewed changes

config/oauth_visitors.go Show resolved Hide resolved

config/oauth_visitors.go Outdated Show resolved Hide resolved

config/oauth_visitors.go Outdated Show resolved Hide resolved

tanmay-db added 3 commits June 14, 2023 17:09

test

a17277f

upd

23dec0b

upd

0e4f319

tanmay-db requested a review from nfx June 14, 2023 15:20

tanmay-db commented Jun 14, 2023

View reviewed changes

nfx approved these changes Jun 14, 2023

View reviewed changes

Merge branch 'main' into refresh-token

e2f8fee

tanmay-db merged commit d5e57f7 into main Jun 14, 2023

nfx deleted the refresh-token branch June 14, 2023 17:23

nfx mentioned this pull request Jun 15, 2023

Release v0.10.0 #520

Merged

pietern mentioned this pull request Sep 15, 2023

Occasional failed request: inner token: Post "https://login.microsoftonline.com/***/oauth2/token": read tcp XXX->YYY: read: connection reset by peer #256

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added retries for getting refreshable token#500

Added retries for getting refreshable token#500
tanmay-db merged 7 commits intomainfrom
refresh-token

tanmay-db commented Jun 14, 2023 •

edited by nfx

Loading

Uh oh!

nfx left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tanmay-db Jun 14, 2023

Uh oh!

nfx Jun 14, 2023

Uh oh!

tanmay-db Jun 14, 2023

Uh oh!

nfx Jun 14, 2023

Uh oh!

codecov-commenter commented Jun 14, 2023

Uh oh!

nfx left a comment

Uh oh!

nfx Jun 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tanmay-db commented Jun 14, 2023 • edited by nfx Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nfx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tanmay-db Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

nfx Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

tanmay-db Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

nfx Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jun 14, 2023

Codecov Report

Uh oh!

nfx left a comment

Choose a reason for hiding this comment

Uh oh!

nfx Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tanmay-db commented Jun 14, 2023 •

edited by nfx

Loading