Skip to content

Commit

Permalink
UX improvements and test coverage fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
garethgeorge committed Jul 11, 2024
1 parent 1ad385f commit 0eddbe0
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 97 deletions.
5 changes: 3 additions & 2 deletions internal/api/backresthandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/garethgeorge/backrest/internal/config"
"github.com/garethgeorge/backrest/internal/oplog"
"github.com/garethgeorge/backrest/internal/orchestrator"
"github.com/garethgeorge/backrest/internal/orchestrator/tasks"
"github.com/garethgeorge/backrest/internal/resticinstaller"
"github.com/garethgeorge/backrest/internal/rotatinglog"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -387,8 +388,8 @@ func TestHookCancellation(t *testing.T) {
}()

_, err := sut.handler.Backup(context.Background(), connect.NewRequest(&types.StringValue{Value: "test"}))
if err != nil {
t.Fatalf("Backup() error = %v", err)
if !errors.Is(err, tasks.ErrTaskCancelled) {
t.Fatalf("Backup() error = %v, want errors.Is(err, tasks.ErrTaskCancelled)", err)
}

// Wait for a hook operation to appear in the oplog
Expand Down
28 changes: 19 additions & 9 deletions internal/hook/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, pare
}

name := fmt.Sprintf("repo/%v/hook/%v", repo.Id, idx)
taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars))
task, err := newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars)
if err != nil {
return nil, err
}
taskSet = append(taskSet, task)
}

for idx, hook := range plan.GetHooks() {
Expand All @@ -43,13 +47,24 @@ func TasksTriggeredByEvent(config *v1.Config, repoID string, planID string, pare
}

name := fmt.Sprintf("plan/%v/hook/%v", plan.Id, idx)
taskSet = append(taskSet, newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars))
task, err := newOneoffRunHookTask(name, config.Instance, repoID, planID, parentOp, time.Now(), hook, event, vars)
if err != nil {
return nil, err
}
taskSet = append(taskSet, task)
}

return taskSet, nil
}

func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1.Operation, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) tasks.Task {
func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1.Operation, at time.Time, hook *v1.Hook, event v1.Hook_Condition, vars interface{}) (tasks.Task, error) {
h, err := types.DefaultRegistry().GetHandler(hook)
if err != nil {
return nil, fmt.Errorf("no handler for hook type %T", hook.Action)
}

title = h.Name() + " hook " + title

return &tasks.GenericOneoffTask{
OneoffTask: tasks.OneoffTask{
BaseTask: tasks.BaseTask{
Expand All @@ -76,11 +91,6 @@ func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1
},
},
Do: func(ctx context.Context, st tasks.ScheduledTask, taskRunner tasks.TaskRunner) error {
h, err := types.DefaultRegistry().GetHandler(hook)
if err != nil {
return err
}

// TODO: this is a hack to get around the fact that vars is an interface{} .
v := reflect.ValueOf(&vars).Elem()
clone := reflect.New(v.Elem().Type()).Elem()
Expand All @@ -95,7 +105,7 @@ func newOneoffRunHookTask(title, instanceID, repoID, planID string, parentOp *v1
}
return nil
},
}
}, nil
}

func firstMatchingCondition(hook *v1.Hook, events []v1.Hook_Condition) v1.Hook_Condition {
Expand Down
85 changes: 0 additions & 85 deletions internal/hook/hook_test.go

This file was deleted.

8 changes: 7 additions & 1 deletion webui/src/components/OperationRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,13 @@ export const OperationRow = ({
});
} else if (operation.op.case === "operationRunHook") {
const hook = operation.op.value;
// TODO: customized view of hook execution info
if (operation.logref) {
bodyItems.push({
key: "logref",
label: "Hook Output",
children: <BigOperationDataVerbatim logref={operation.logref} />,
});
}
}

if (hookOperations) {
Expand Down

0 comments on commit 0eddbe0

Please sign in to comment.