From c7a85f73dadd600e6ab8166efbdd224889457124 Mon Sep 17 00:00:00 2001 From: Tim Barry <21149133+tim-barry@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:55:51 -0800 Subject: [PATCH] Apply suggestions from code review Update/clarify doc comments Co-authored-by: Jordan Schalm --- engine/execution/ingestion/uploader/uploader.go | 3 ++- module/component/component.go | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/engine/execution/ingestion/uploader/uploader.go b/engine/execution/ingestion/uploader/uploader.go index e888d9589aa..db8bbe93189 100644 --- a/engine/execution/ingestion/uploader/uploader.go +++ b/engine/execution/ingestion/uploader/uploader.go @@ -55,7 +55,6 @@ type AsyncUploader struct { queue chan *execution.ComputationResult cm *component.ComponentManager component.Component - // TODO Replace fifoqueue with channel, and make Upload() blocking } // UploadWorker implements a component worker which asynchronously uploads computation results @@ -108,6 +107,8 @@ func (a *AsyncUploader) UploadTask(ctx context.Context, computationResult *execu return retry.RetryableError(err) }) + // We only log upload errors here because the errors originate from an external cloud provider + // and the upload success is not critical to correct continued operation of the node if err != nil { a.log.Error().Err(err). Hex("block_id", logging.Entity(computationResult.ExecutableBlock)). diff --git a/module/component/component.go b/module/component/component.go index 3f813ad0612..e78b24fffec 100644 --- a/module/component/component.go +++ b/module/component/component.go @@ -23,7 +23,7 @@ var ErrComponentShutdown = fmt.Errorf("component has already shut down") type Component interface { module.Startable // Ready returns a ready channel that is closed once startup has completed. - // Unlike the previous ReadyDoneAware interface, it has no effect on the state of the component, + // Unlike the previous [module.ReadyDoneAware] interface, it has no effect on the state of the component, // and only exposes information about the component's state. // To start the component, instead use the Start() method. // Note that the ready channel may never close if errors are encountered during startup, @@ -32,10 +32,10 @@ type Component interface { Ready() <-chan struct{} // Done returns a done channel that is closed once shutdown has completed. - // Unlike the previous ReadyDoneAware interface, it has no effect on the state of the component, + // Unlike the previous [module.ReadyDoneAware] interface, it has no effect on the state of the component, // and only exposes information about the component's state. // To shutdown the component, instead cancel the context that was passed to Start(). - // Note that the done channel should be closed even if errors are encountered during shutdown. + // Implementations must close the done channel even if errors are encountered during shutdown. // This should be an idempotent method. Done() <-chan struct{} }