fix: UI and code quality improvements

garethgeorge · Dec 26, 2023 · c5e435d · c5e435d
1 parent 6bfe769
commit c5e435d
Show file tree

Hide file tree

Showing 12 changed files with 67 additions and 48 deletions.
diff --git a/internal/api/server.go b/internal/api/server.go
@@ -254,7 +254,7 @@ func (s *Server) Backup(ctx context.Context, req *connect.Request[types.StringVa
 	if err != nil {
 		return nil, fmt.Errorf("failed to get plan %q: %w", req.Msg.Value, err)
 	}
-	s.orchestrator.ScheduleTask(orchestrator.NewOneofBackupTask(s.orchestrator, plan, time.Now()), orchestrator.TaskPriorityInteractive)
+	s.orchestrator.ScheduleTask(orchestrator.NewOneoffBackupTask(s.orchestrator, plan, time.Now()), orchestrator.TaskPriorityInteractive)
 	return connect.NewResponse(&emptypb.Empty{}), nil
 }
 
@@ -266,8 +266,8 @@ func (s *Server) Forget(ctx context.Context, req *connect.Request[types.StringVa
 
 	at := time.Now()
 
-	s.orchestrator.ScheduleTask(orchestrator.NewOneofForgetTask(s.orchestrator, plan, "", at), orchestrator.TaskPriorityInteractive+orchestrator.TaskPriorityForget)
-	s.orchestrator.ScheduleTask(orchestrator.NewOneofIndexSnapshotsTask(s.orchestrator, plan, at), orchestrator.TaskPriorityInteractive+orchestrator.TaskPriorityIndexSnapshots)
+	s.orchestrator.ScheduleTask(orchestrator.NewOneoffForgetTask(s.orchestrator, plan, "", at), orchestrator.TaskPriorityInteractive+orchestrator.TaskPriorityForget)
+	s.orchestrator.ScheduleTask(orchestrator.NewOneoffIndexSnapshotsTask(s.orchestrator, plan, at), orchestrator.TaskPriorityInteractive+orchestrator.TaskPriorityIndexSnapshots)
 
 	return connect.NewResponse(&emptypb.Empty{}), nil
 }
@@ -279,7 +279,7 @@ func (s *Server) Prune(ctx context.Context, req *connect.Request[types.StringVal
 	}
 
 	at := time.Now()
-	s.orchestrator.ScheduleTask(orchestrator.NewOneofPruneTask(s.orchestrator, plan, "", at, true), orchestrator.TaskPriorityInteractive+orchestrator.TaskPriorityPrune)
+	s.orchestrator.ScheduleTask(orchestrator.NewOneoffPruneTask(s.orchestrator, plan, "", at, true), orchestrator.TaskPriorityInteractive+orchestrator.TaskPriorityPrune)
 
 	return connect.NewResponse(&emptypb.Empty{}), nil
 }
@@ -302,7 +302,7 @@ func (s *Server) Restore(ctx context.Context, req *connect.Request[v1.RestoreSna
 
 	at := time.Now()
 
-	s.orchestrator.ScheduleTask(orchestrator.NewOneofRestoreTask(s.orchestrator, orchestrator.RestoreTaskOpts{
+	s.orchestrator.ScheduleTask(orchestrator.NewOneoffRestoreTask(s.orchestrator, orchestrator.RestoreTaskOpts{
 		Plan:       plan,
 		SnapshotId: req.Msg.SnapshotId,
 		Path:       req.Msg.Path,

diff --git a/internal/orchestrator/taskbackup.go b/internal/orchestrator/taskbackup.go
@@ -42,7 +42,7 @@ func NewScheduledBackupTask(orchestrator *Orchestrator, plan *v1.Plan) (*BackupT
 	}, nil
 }
 
-func NewOneofBackupTask(orchestrator *Orchestrator, plan *v1.Plan, at time.Time) *BackupTask {
+func NewOneoffBackupTask(orchestrator *Orchestrator, plan *v1.Plan, at time.Time) *BackupTask {
 	didOnce := false
 	return &BackupTask{
 		name: fmt.Sprintf("onetime backup for plan %q", plan.Id),
@@ -142,11 +142,11 @@ func backupHelper(ctx context.Context, orchestrator *Orchestrator, plan *v1.Plan
 	// schedule followup tasks
 	at := time.Now()
 	if plan.Retention != nil {
-		orchestrator.ScheduleTask(NewOneofForgetTask(orchestrator, plan, op.SnapshotId, at), TaskPriorityForget)
+		orchestrator.ScheduleTask(NewOneoffForgetTask(orchestrator, plan, op.SnapshotId, at), TaskPriorityForget)
 	}
 
-	orchestrator.ScheduleTask(NewOneofIndexSnapshotsTask(orchestrator, plan, at), TaskPriorityIndexSnapshots)
-	orchestrator.ScheduleTask(NewOneofStatsTask(orchestrator, plan, op.SnapshotId, at), TaskPriorityStats)
+	orchestrator.ScheduleTask(NewOneoffIndexSnapshotsTask(orchestrator, plan, at), TaskPriorityIndexSnapshots)
+	orchestrator.ScheduleTask(NewOneoffStatsTask(orchestrator, plan, op.SnapshotId, at), TaskPriorityStats)
 
 	return nil
 }
diff --git a/internal/orchestrator/taskforget.go b/internal/orchestrator/taskforget.go
@@ -23,7 +23,7 @@ type ForgetTask struct {
 
 var _ Task = &ForgetTask{}
 
-func NewOneofForgetTask(orchestrator *Orchestrator, plan *v1.Plan, linkSnapshot string, at time.Time) *ForgetTask {
+func NewOneoffForgetTask(orchestrator *Orchestrator, plan *v1.Plan, linkSnapshot string, at time.Time) *ForgetTask {
 	return &ForgetTask{
 		TaskWithOperation: TaskWithOperation{
 			orch: orchestrator,
@@ -102,7 +102,7 @@ func (t *ForgetTask) Run(ctx context.Context) error {
 		}
 
 		if len(forgot) > 0 {
-			t.orch.ScheduleTask(NewOneofPruneTask(t.orch, t.plan, op.SnapshotId, time.Now(), false), TaskPriorityPrune)
+			t.orch.ScheduleTask(NewOneoffPruneTask(t.orch, t.plan, op.SnapshotId, time.Now(), false), TaskPriorityPrune)
 		}
 
 		return err

diff --git a/internal/orchestrator/taskindexsnapshots.go b/internal/orchestrator/taskindexsnapshots.go
@@ -21,7 +21,7 @@ type IndexSnapshotsTask struct {
 
 var _ Task = &IndexSnapshotsTask{}
 
-func NewOneofIndexSnapshotsTask(orchestrator *Orchestrator, plan *v1.Plan, at time.Time) *IndexSnapshotsTask {
+func NewOneoffIndexSnapshotsTask(orchestrator *Orchestrator, plan *v1.Plan, at time.Time) *IndexSnapshotsTask {
 	return &IndexSnapshotsTask{
 		orchestrator: orchestrator,
 		plan:         plan,

diff --git a/internal/orchestrator/taskprune.go b/internal/orchestrator/taskprune.go
@@ -23,7 +23,7 @@ type PruneTask struct {
 
 var _ Task = &PruneTask{}
 
-func NewOneofPruneTask(orchestrator *Orchestrator, plan *v1.Plan, linkSnapshot string, at time.Time, force bool) *PruneTask {
+func NewOneoffPruneTask(orchestrator *Orchestrator, plan *v1.Plan, linkSnapshot string, at time.Time, force bool) *PruneTask {
 	return &PruneTask{
 		TaskWithOperation: TaskWithOperation{
 			orch: orchestrator,
@@ -60,7 +60,7 @@ func (t *PruneTask) Next(now time.Time) *time.Time {
 
 func (t *PruneTask) getNextPruneTime(repo *RepoOrchestrator, policy *v1.PrunePolicy) (time.Time, error) {
 	var lastPruneTime time.Time
-	t.orch.OpLog.ForEachByRepo(t.plan.Repo, indexutil.CollectLastN(1000), func(op *v1.Operation) error {
+	t.orch.OpLog.ForEachByRepo(t.plan.Repo, indexutil.CollectLastN(100), func(op *v1.Operation) error {
 		if _, ok := op.Op.(*v1.Operation_OperationPrune); ok {
 			lastPruneTime = time.Unix(0, op.UnixTimeStartMs*int64(time.Millisecond))
 		}
@@ -145,6 +145,8 @@ func (t *PruneTask) Run(ctx context.Context) error {
 			},
 		}
 
+		// Schedule a task to update persisted stats for the repo
+
 		return nil
 	})
 }

diff --git a/internal/orchestrator/taskrestore.go b/internal/orchestrator/taskrestore.go
@@ -25,7 +25,7 @@ type RestoreTask struct {
 
 var _ Task = &RestoreTask{}
 
-func NewOneofRestoreTask(orchestrator *Orchestrator, opts RestoreTaskOpts, at time.Time) *RestoreTask {
+func NewOneoffRestoreTask(orchestrator *Orchestrator, opts RestoreTaskOpts, at time.Time) *RestoreTask {
 	return &RestoreTask{
 		TaskWithOperation: TaskWithOperation{
 			orch: orchestrator,

diff --git a/internal/orchestrator/taskstats.go b/internal/orchestrator/taskstats.go
@@ -20,7 +20,7 @@ type StatsTask struct {
 
 var _ Task = &ForgetTask{}
 
-func NewOneofStatsTask(orchestrator *Orchestrator, plan *v1.Plan, linkSnapshot string, at time.Time) *StatsTask {
+func NewOneoffStatsTask(orchestrator *Orchestrator, plan *v1.Plan, linkSnapshot string, at time.Time) *StatsTask {
 	return &StatsTask{
 		TaskWithOperation: TaskWithOperation{
 			orch: orchestrator,

diff --git a/webui/src/components/OperationList.tsx b/webui/src/components/OperationList.tsx
@@ -209,12 +209,7 @@ export const OperationRow = ({
 
   let body: React.ReactNode | undefined;
 
-  if (
-    operation.displayMessage &&
-    operation.status === OperationStatus.STATUS_ERROR
-  ) {
-    body = <pre>{operation.displayMessage}</pre>;
-  } else if (operation.op.case === "operationBackup") {
+  if (operation.op.case === "operationBackup") {
     const backupOp = operation.op.value;
     body = (
       <>
@@ -274,6 +269,15 @@ export const OperationRow = ({
     );
   }
 
+  if (operation.displayMessage) {
+    body = (
+      <>
+        <pre>{details.state}: {operation.displayMessage}</pre>
+        {body}
+      </>
+    );
+  }
+
   return (
     <List.Item>
       <List.Item.Meta title={title} avatar={avatar} description={body} />

diff --git a/webui/src/constants.ts b/webui/src/constants.ts
@@ -1,2 +1 @@
-export const API_PREFIX = "/api";
 export const MAX_OPERATION_HISTORY = 10000;
diff --git a/webui/src/lib/formatting.ts b/webui/src/lib/formatting.ts
@@ -29,9 +29,10 @@ export const formatTime = (time: number | string | Date) => {
   const hours = d.getUTCHours() % 12 == 0 ? 12 : d.getUTCHours() % 12;
   const minutes =
     d.getUTCMinutes() < 10 ? "0" + d.getUTCMinutes() : d.getUTCMinutes();
-  return `${isoStr.substring(0, 10)} at ${hours}:${minutes} ${
-    d.getUTCHours() > 12 ? "PM" : "AM"
-  }`;
+  const seconds =
+    d.getUTCSeconds() < 10 ? "0" + d.getUTCSeconds() : d.getUTCSeconds();
+  return `${isoStr.substring(0, 10)} at ${hours}:${minutes}:${seconds} ${d.getUTCHours() > 12 ? "PM" : "AM"
+    }`;
 };
 
 export const localISOTime = (time: number | string | Date) => {

diff --git a/webui/src/state/oplog.ts b/webui/src/state/oplog.ts
@@ -153,8 +153,16 @@ export class BackupInfoCollector {
     // use the latest status that is not cancelled.
     let statusIdx = operations.length - 1;
     let status = OperationStatus.STATUS_SYSTEM_CANCELLED;
-    while (statusIdx !== -1 && (shouldHideStatus(status) || status === OperationStatus.STATUS_PENDING)) {
-      status = operations[statusIdx].status!;
+    while (statusIdx !== -1) {
+      const curStatus = operations[statusIdx].status;
+      if (
+        shouldHideStatus(status) ||
+        status === OperationStatus.STATUS_PENDING ||
+        curStatus === OperationStatus.STATUS_ERROR ||
+        curStatus === OperationStatus.STATUS_WARNING
+      ) {
+        status = operations[statusIdx].status;
+      }
       statusIdx--;
     }
 

diff --git a/webui/src/views/RepoView.tsx b/webui/src/views/RepoView.tsx
@@ -1,6 +1,6 @@
 import React, { useEffect, useState } from "react";
 import { Repo } from "../../gen/ts/v1/config_pb";
-import { Col, Empty, Flex, Row, Tabs, Typography } from "antd";
+import { Col, Empty, Flex, Row, Spin, Tabs, Typography } from "antd";
 import { useRecoilValue } from "recoil";
 import { configState } from "../state/config";
 import { useAlertApi } from "../components/Alerts";
@@ -14,9 +14,11 @@ import { formatBytes } from "../lib/formatting";
 
 export const RepoView = ({ repo }: React.PropsWithChildren<{ repo: Repo }>) => {
   const alertsApi = useAlertApi()!;
+  const [loading, setLoading] = useState(true);
   const [stats, setStats] = useState<RepoStats | null>(null);
 
   useEffect(() => {
+    setLoading(true);
     setStats(null);
     getOperations(new GetOperationsRequest({ repoId: repo.id!, lastN: BigInt(10) })).then((operations) => {
       for (const op of operations) {
@@ -29,10 +31,11 @@ export const RepoView = ({ repo }: React.PropsWithChildren<{ repo: Repo }>) => {
       }
     }).catch((e) => {
       console.error(e);
+    }).finally(() => {
+      setLoading(false);
     });
   }, [repo.id]);
 
-
   // Gracefully handle deletions by checking if the plan is still in the config.
   const config = useRecoilValue(configState);
   let repoInConfig = config.repos?.find((p) => p.id === repo.id);
@@ -41,8 +44,8 @@ export const RepoView = ({ repo }: React.PropsWithChildren<{ repo: Repo }>) => {
   }
   repo = repoInConfig;
 
-  if (!stats) {
-    return <Empty description="No stats available. Run a backup." />;
+  if (loading) {
+    return <Spin />;
   }
 
   const items = [
@@ -52,22 +55,24 @@ export const RepoView = ({ repo }: React.PropsWithChildren<{ repo: Repo }>) => {
       children: (
         <>
           <h3>Repo Stats</h3>
-          <Row>
-            <Col style={{ paddingRight: "20px" }}>
-              <p><strong>Total Size: </strong></p>
-              <p><strong>Total Size Uncompressed: </strong></p>
-              <p><strong>Blob Count: </strong></p>
-              <p><strong>Snapshot Count: </strong></p>
-              <p><strong>Compression Ratio: </strong></p>
-            </Col>
-            <Col>
-              <p>{formatBytes(Number(stats.totalSize))}</p>
-              <p>{formatBytes(Number(stats.totalUncompressedSize))}</p>
-              <p>{Number(stats.totalBlobCount)} blobs</p>
-              <p>{Number(stats.snapshotCount)} snapshots</p>
-              <p>{Math.round(stats.compressionRatio * 1000) / 1000}</p>
-            </Col>
-          </Row>
+          {stats === null ? <Empty description="No data. Have you run a backup yet?" /> :
+            <Row>
+              <Col style={{ paddingRight: "20px" }}>
+                <p><strong>Total Size: </strong></p>
+                <p><strong>Total Size Uncompressed: </strong></p>
+                <p><strong>Blob Count: </strong></p>
+                <p><strong>Snapshot Count: </strong></p>
+                <p><strong>Compression Ratio: </strong></p>
+              </Col>
+              <Col>
+                <p>{formatBytes(Number(stats.totalSize))}</p>
+                <p>{formatBytes(Number(stats.totalUncompressedSize))}</p>
+                <p>{Number(stats.totalBlobCount)} blobs</p>
+                <p>{Number(stats.snapshotCount)} snapshots</p>
+                <p>{Math.round(stats.compressionRatio * 1000) / 1000}</p>
+              </Col>
+            </Row>
+          }
         </>
       ),
     },
Original file line number	Diff line number	Diff line change
		@@ -1,2 +1 @@
		export const API_PREFIX = "/api";
		export const MAX_OPERATION_HISTORY = 10000;