diff --git a/CHANGELOG.md b/CHANGELOG.md index 192d57e..de95b89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ All notable changes to this project will be documented in this file. See updating [Changelog example here](https://keepachangelog.com/en/1.0.0/). +## 0.21.0 + +### Added + +* Handling API rate limits to wait or retry calls depending on the current window or remaining limits. + +### Fixed + +* Removed extra API call when retrieving latest backup and import Statuses. + ## 0.8.0 ### Added diff --git a/internal/model.go b/internal/model.go index 9e8fc42..930b2b0 100644 --- a/internal/model.go +++ b/internal/model.go @@ -36,8 +36,8 @@ type Error struct { Status *string `json:"status,omitempty"` } -func (o Error) String() string { - return ToString(o) +func (e Error) String() string { + return ToString(e) } func (e *Error) StatusCode() string { diff --git a/internal/service.go b/internal/service.go index 8c8960f..1be8795 100644 --- a/internal/service.go +++ b/internal/service.go @@ -115,56 +115,14 @@ func (a *api) waitForTaskToComplete(ctx context.Context, id string) (*Task, erro a.logger.Println(err) })) if err != nil { - return nil, err + return task, err } return task, nil } func (a *api) WaitForTask(ctx context.Context, id string) (*Task, error) { - var task *Task - notFoundCount := 0 - err := retry.Do( - func() error { - var err error - task, err = a.get(ctx, id) - if err != nil { - // An error is a terminal state (any repeated pre-task 404s will have been exhausted by this point) - return nil - } - - status := redis.StringValue(task.Status) - - if _, ok := processingStates[status]; !ok { - // The task is no longer processing for whatever reason - return nil - } - - return fmt.Errorf("task %s not processed yet: %s", id, status) - }, - retry.Attempts(math.MaxUint16), - retry.Delay(1*time.Second), - retry.MaxDelay(30*time.Second), - retry.RetryIf(func(err error) bool { - if !retry.IsRecoverable(err) { - return false - } - if _, ok := err.(*taskNotFoundError); ok { - notFoundCount++ - if notFoundCount > max404Errors { - return false - } - } - return true - }), - retry.LastErrorOnly(true), retry.Context(ctx), retry.OnRetry(func(_ uint, err error) { - a.logger.Println(err) - })) - if err != nil { - return nil, err - } - - return task, nil + return a.waitForTaskToComplete(ctx, id) } func (a *api) get(ctx context.Context, id string) (*Task, error) { diff --git a/latest_backups_test.go b/latest_backups_test.go index ff17e1a..5a5b793 100644 --- a/latest_backups_test.go +++ b/latest_backups_test.go @@ -57,29 +57,6 @@ func TestGetLatestBackup(t *testing.T) { ] }`, ), - getRequest( - t, - "/tasks/50ec6172-8475-4ef6-8b3c-d61e688d8fe5", - `{ - "taskId": "50ec6172-8475-4ef6-8b3c-d61e688d8fe5", - "commandType": "databaseBackupStatusRequest", - "status": "processing-completed", - "description": "Request processing completed successfully and its resources are now being provisioned / de-provisioned.", - "timestamp": "2024-04-15T09:08:07.537915Z", - "response": { - "resourceId": 51051292, - "additionalResourceId": 12, - "resource": {} - }, - "links": [ - { - "href": "https://api-staging.qa.redislabs.com/v1/tasks/50ec6172-8475-4ef6-8b3c-d61e688d8fe5", - "type": "GET", - "rel": "self" - } - ] - }`, - ), )) subject, err := clientFromTestServer(server, "key", "secret") @@ -135,29 +112,6 @@ func TestGetFixedLatestBackup(t *testing.T) { ] }`, ), - getRequest( - t, - "/tasks/ce2cbfea-9b15-4250-a516-f014161a8dd3", - `{ - "taskId": "ce2cbfea-9b15-4250-a516-f014161a8dd3", - "commandType": "databaseBackupStatusRequest", - "status": "processing-completed", - "description": "Request processing completed successfully and its resources are now being provisioned / de-provisioned.", - "timestamp": "2024-04-15T09:52:26.101936Z", - "response": { - "resource": { - "status": "success" - } - }, - "links": [ - { - "href": "https://api-staging.qa.redislabs.com/v1/tasks/ce2cbfea-9b15-4250-a516-f014161a8dd3", - "type": "GET", - "rel": "self" - } - ] - }`, - ), )) subject, err := clientFromTestServer(server, "key", "secret") @@ -230,36 +184,26 @@ func TestGetAALatestBackup(t *testing.T) { ] }`, ), - getRequest( - t, - "/tasks/ce2cbfea-9b15-4250-a516-f014161a8dd3", - `{ - "taskId": "ce2cbfea-9b15-4250-a516-f014161a8dd3", - "commandType": "databaseBackupStatusRequest", - "status": "processing-error", - "description": "Task request failed during processing. See error information for failure details.", - "timestamp": "2024-04-15T09:52:26.101936Z", - "response": { - "error": { - "type": "DATABASE_BACKUP_DISABLED", - "status": "400 BAD_REQUEST", - "description": "Database backup is disabled" - } - }, - "links": [ - { - "href": "https://api-staging.qa.redislabs.com/v1/tasks/ce2cbfea-9b15-4250-a516-f014161a8dd3", - "type": "GET", - "rel": "self" - } - ] - }`, - ), )) subject, err := clientFromTestServer(server, "key", "secret") require.NoError(t, err) - _, err = subject.LatestBackup.GetActiveActive(context.TODO(), 12, 34, "eu-west-2") + actual, err := subject.LatestBackup.GetActiveActive(context.TODO(), 12, 34, "eu-west-2") require.NoError(t, err) + + assert.Equal(t, &latest_backups.LatestBackupStatus{ + CommandType: redis.String("databaseBackupStatusRequest"), + Description: redis.String("Task request failed during processing. See error information for failure details."), + Status: redis.String("processing-error"), + ID: redis.String("ce2cbfea-9b15-4250-a516-f014161a8dd3"), + Response: &latest_backups.Response{ + Error: &latest_backups.Error{ + Type: redis.String("DATABASE_BACKUP_DISABLED"), + Description: redis.String("Database backup is disabled"), + Status: redis.String("400 BAD_REQUEST"), + }, + }, + }, actual) + } diff --git a/latest_imports_test.go b/latest_imports_test.go index 2c28b15..8bb506f 100644 --- a/latest_imports_test.go +++ b/latest_imports_test.go @@ -61,38 +61,27 @@ func TestGetLatestImportTooEarly(t *testing.T) { ] }`, ), - getRequest( - t, - "/tasks/1dfd6084-21df-40c6-829c-e9b4790e207e", - `{ - "taskId": "1dfd6084-21df-40c6-829c-e9b4790e207e", - "commandType": "databaseImportStatusRequest", - "status": "processing-error", - "description": "Task request failed during processing. See error information for failure details.", - "timestamp": "2024-04-15T10:19:07.331898Z", - "response": { - "error": { - "type": "SUBSCRIPTION_NOT_ACTIVE", - "status": "403 FORBIDDEN", - "description": "Cannot preform any actions for subscription that is not in an active state" - } - }, - "links": [ - { - "href": "https://api-staging.qa.redislabs.com/v1/tasks/1dfd6084-21df-40c6-829c-e9b4790e207e", - "type": "GET", - "rel": "self" - } - ] - }`, - ), )) subject, err := clientFromTestServer(server, "key", "secret") require.NoError(t, err) - _, err = subject.LatestImport.Get(context.TODO(), 12, 34) + actual, err := subject.LatestImport.Get(context.TODO(), 12, 34) require.NoError(t, err) + + assert.Equal(t, &latest_imports.LatestImportStatus{ + CommandType: redis.String("databaseImportStatusRequest"), + Description: redis.String("Task request failed during processing. See error information for failure details."), + Status: redis.String("processing-error"), + ID: redis.String("1dfd6084-21df-40c6-829c-e9b4790e207e"), + Response: &latest_imports.Response{ + Error: &latest_imports.Error{ + Type: redis.String("SUBSCRIPTION_NOT_ACTIVE"), + Description: redis.String("Cannot preform any actions for subscription that is not in an active state"), + Status: redis.String("403 FORBIDDEN"), + }, + }, + }, actual) } func TestGetFixedLatestImport(t *testing.T) { @@ -143,31 +132,6 @@ func TestGetFixedLatestImport(t *testing.T) { ] }`, ), - getRequest( - t, - "/tasks/e9232e43-3781-4263-a38e-f4d150e03475", - `{ - "taskId": "e9232e43-3781-4263-a38e-f4d150e03475", - "commandType": "databaseImportStatusRequest", - "status": "processing-completed", - "description": "Request processing completed successfully and its resources are now being provisioned / de-provisioned.", - "timestamp": "2024-04-15T10:44:35.225468Z", - "response": { - "resourceId": 51051302, - "additionalResourceId": 110777, - "resource": { - "status": "importing" - } - }, - "links": [ - { - "href": "https://api-staging.qa.redislabs.com/v1/tasks/e9232e43-3781-4263-a38e-f4d150e03475", - "type": "GET", - "rel": "self" - } - ] - }`, - ), )) subject, err := clientFromTestServer(server, "key", "secret") @@ -251,43 +215,6 @@ func TestGetLatestImport(t *testing.T) { ] }`, ), - getRequest( - t, - "/tasks/e9232e43-3781-4263-a38e-f4d150e03475", - `{ - "taskId": "e9232e43-3781-4263-a38e-f4d150e03475", - "commandType": "databaseImportStatusRequest", - "status": "processing-completed", - "description": "Request processing completed successfully and its resources are now being provisioned / de-provisioned.", - "timestamp": "2024-04-15T10:44:35.225468Z", - "response": { - "resourceId": 51051302, - "additionalResourceId": 110777, - "resource": { - "failureReason": "file-corrupted", - "failureReasonParams": [ - { - "key": "bytes_configured_bdb_limit", - "value": "1234" - }, - { - "key": "bytes_of_expected_dataset", - "value": "5678" - } - ], - "lastImportTime": "2024-05-21T10:36:26Z", - "status": "failed" - } - }, - "links": [ - { - "href": "https://api-staging.qa.redislabs.com/v1/tasks/e9232e43-3781-4263-a38e-f4d150e03475", - "type": "GET", - "rel": "self" - } - ] - }`, - ), )) subject, err := clientFromTestServer(server, "key", "secret") diff --git a/service/latest_backups/service.go b/service/latest_backups/service.go index d06013e..686076e 100644 --- a/service/latest_backups/service.go +++ b/service/latest_backups/service.go @@ -2,6 +2,8 @@ package latest_backups import ( "context" + "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -15,7 +17,7 @@ type HttpClient interface { } type TaskWaiter interface { - Wait(ctx context.Context, id string) error + WaitForTask(ctx context.Context, id string) (*internal.Task, error) } type Log interface { @@ -68,23 +70,21 @@ func (a *API) GetActiveActive(ctx context.Context, subscription int, database in a.logger.Printf("Waiting for backup status request %d to complete", task.ID) - err = a.taskWaiter.Wait(ctx, *task.ID) - - a.logger.Printf("Backup status request %d completed, possibly with error", task.ID, err) - - var backupStatusTask *LatestBackupStatus - err = a.client.Get(ctx, - fmt.Sprintf("retrieve completed backup status task %d", task.ID), - "/tasks/"+*task.ID, - &backupStatusTask, - ) - + taskResp, err := a.taskWaiter.WaitForTask(ctx, *task.ID) if err != nil { + var iErr *internal.Error + if errors.As(err, &iErr) && taskResp != nil { + backupStatusTask, err := createLatestBackupStatusFromTask(taskResp) + if err != nil { + return nil, err + } + return backupStatusTask, nil + } return nil, wrap404ErrorActiveActive(subscription, database, region, fmt.Errorf("failed to retrieve completed backup status %d: %w", task.ID, err)) } - return backupStatusTask, nil + return createLatestBackupStatusFromTask(taskResp) } func (a *API) get(ctx context.Context, message string, address string) (*LatestBackupStatus, error) { @@ -96,22 +96,20 @@ func (a *API) get(ctx context.Context, message string, address string) (*LatestB a.logger.Printf("Waiting for backup status request %d to complete", task.ID) - err = a.taskWaiter.Wait(ctx, *task.ID) - - a.logger.Printf("Backup status request %d completed, possibly with error", task.ID, err) - - var backupStatusTask *LatestBackupStatus - err = a.client.Get(ctx, - fmt.Sprintf("retrieve completed backup status task %d", task.ID), - "/tasks/"+*task.ID, - &backupStatusTask, - ) - + taskResp, err := a.taskWaiter.WaitForTask(ctx, *task.ID) if err != nil { + var iErr *internal.Error + if errors.As(err, &iErr) && taskResp != nil { + backupStatusTask, err := createLatestBackupStatusFromTask(taskResp) + if err != nil { + return nil, err + } + return backupStatusTask, nil + } return nil, fmt.Errorf("failed to retrieve completed backup status %d: %w", task.ID, err) } - return backupStatusTask, nil + return createLatestBackupStatusFromTask(taskResp) } func wrap404Error(subId int, dbId int, err error) error { @@ -127,3 +125,34 @@ func wrap404ErrorActiveActive(subId int, dbId int, region string, err error) err } return err } + +func createLatestBackupStatusFromTask(task *internal.Task) (*LatestBackupStatus, error) { + latestBackupStatus := &LatestBackupStatus{} + if task != nil { + latestBackupStatus.CommandType = task.CommandType + latestBackupStatus.Description = task.Description + latestBackupStatus.Status = task.Status + latestBackupStatus.ID = task.ID + if task.Response != nil { + latestBackupStatus.Response = &Response{ + ID: task.Response.ID, + } + if task.Response.Error != nil { + latestBackupStatus.Response.Error = &Error{ + Type: task.Response.Error.Type, + Description: task.Response.Error.Description, + Status: task.Response.Error.Status, + } + } + + if task.Response.Resource != nil { + latestBackupStatus.Response.Resource = &Resource{} + err := json.Unmarshal(*task.Response.Resource, latestBackupStatus.Response.Resource) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal task response: %w", err) + } + } + } + } + return latestBackupStatus, nil +} diff --git a/service/latest_imports/service.go b/service/latest_imports/service.go index 13a934d..af8c20b 100644 --- a/service/latest_imports/service.go +++ b/service/latest_imports/service.go @@ -2,6 +2,8 @@ package latest_imports import ( "context" + "encoding/json" + "errors" "fmt" "net/http" @@ -13,7 +15,7 @@ type HttpClient interface { } type TaskWaiter interface { - Wait(ctx context.Context, id string) error + WaitForTask(ctx context.Context, id string) (*internal.Task, error) } type Log interface { @@ -59,22 +61,26 @@ func (a *API) get(ctx context.Context, message string, address string) (*LatestI a.logger.Printf("Waiting for import status request %d to complete", task.ID) - err = a.taskWaiter.Wait(ctx, *task.ID) - - a.logger.Printf("Import status request %d completed, possibly with error", task.ID, err) + taskResp, err := a.taskWaiter.WaitForTask(ctx, *task.ID) + if err != nil { + var iErr *internal.Error + if errors.As(err, &iErr) && taskResp != nil { + importStatusTask, err := createLatestImportStatusFromTask(taskResp) + if err != nil { + return nil, err + } + return importStatusTask, nil + } + return nil, fmt.Errorf("failed to retrieve completed backup status %d: %w", task.ID, err) + } - var importStatusTask *LatestImportStatus - err = a.client.Get(ctx, - fmt.Sprintf("retrieve completed import status task %d", task.ID), - "/tasks/"+*task.ID, - &importStatusTask, - ) + a.logger.Printf("Import status request %d completed, possibly with error: %v", task.ID, err) if err != nil { return nil, fmt.Errorf("failed to retrieve completed import status %d: %w", task.ID, err) } - return importStatusTask, nil + return createLatestImportStatusFromTask(taskResp) } func wrap404Error(subId int, dbId int, err error) error { @@ -83,3 +89,34 @@ func wrap404Error(subId int, dbId int, err error) error { } return err } + +func createLatestImportStatusFromTask(task *internal.Task) (*LatestImportStatus, error) { + latestImportStatus := &LatestImportStatus{} + if task != nil { + latestImportStatus.CommandType = task.CommandType + latestImportStatus.Description = task.Description + latestImportStatus.Status = task.Status + latestImportStatus.ID = task.ID + if task.Response != nil { + latestImportStatus.Response = &Response{ + ID: task.Response.ID, + } + if task.Response.Error != nil { + latestImportStatus.Response.Error = &Error{ + Type: task.Response.Error.Type, + Description: task.Response.Error.Description, + Status: task.Response.Error.Status, + } + } + + if task.Response.Resource != nil { + latestImportStatus.Response.Resource = &Resource{} + err := json.Unmarshal(*task.Response.Resource, latestImportStatus.Response.Resource) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal task response: %w", err) + } + } + } + } + return latestImportStatus, nil +}