Skip to content

Commit

Permalink
Return non-0 exit code from copilot upload failure
Browse files Browse the repository at this point in the history
 - The current copilot behavior is to:

   * try to upload the specified file
   * if an error occurs, try to write an error file instead
     * if the error file write succeeds, return success -- otherwise error

   This behavior is not ideal, because it suppresses real issues like
   the inability to find the expected output file. While a case can be
   made that the sidecar succeeded in handling its responsibility,
   important information is lost in the process that is useful to other
   containers in the job.

   Generallyk, it's better to consider the inability to
   find the original file a failure of the job to produce the expected
   file in the expected location (useful especially for container tasks).
   That means that a failure to upload the file should also return a
   non-0 exit code.

 - The new behavior is therefore:

   * try to upload the specified file
   * if an error occurs, try to write an error file instead
     * if the error file write succeeds, return the original error
     * if the error file write fails, return a joined version of the
       errors

Signed-off-by: ddl-ebrown <[email protected]>
  • Loading branch information
ddl-ebrown committed Jun 3, 2024
1 parent 75b33f8 commit 49ddaa8
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions flytecopilot/cmd/sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"context"
stderrors "errors"
"fmt"
"time"

Expand Down Expand Up @@ -144,10 +145,13 @@ func (u *UploadOptions) Sidecar(ctx context.Context) error {

if err := u.uploader(ctx); err != nil {
logger.Errorf(ctx, "Uploading failed, err %s", err)
if err := u.UploadError(ctx, "OutputUploadFailed", err, storage.DataReference(u.remoteOutputsPrefix)); err != nil {
logger.Errorf(ctx, "Failed to write error document, err :%s", err)
return err
if uerr := u.UploadError(ctx, "OutputUploadFailed", err, storage.DataReference(u.remoteOutputsPrefix)); uerr != nil {
logger.Errorf(ctx, "Failed to write error document, err :%s", uerr)
return stderrors.Join(uerr, err)
}

// failure to upload should still return err exit code from process
return err
}
return nil
}
Expand Down

0 comments on commit 49ddaa8

Please sign in to comment.