Change to approprite error msg when directory not found in cache intel #73
Conversation
cache/rebuilder.go
Outdated
| @@ -140,7 +140,7 @@ func (r rebuilder) rebuild(src, dst string) (err error) { | |||
| tr := io.TeeReader(pr, sw) | |||
|
|
|||
| if err := r.s.Put(dst, tr); err != nil { | |||
| err = fmt.Errorf("upload file, pipe reader failed, %w", err) | |||
| err = fmt.Errorf("failure in uploading file to archived directory, %w", err) | |||
There was a problem hiding this comment.
failed to upload file to cache storage
There was a problem hiding this comment.
Doe this print the archive name (and associated cache path)?
There was a problem hiding this comment.
No, it prints just the cache path.
cache/restorer.go
Outdated
| @@ -120,7 +120,7 @@ func (r restorer) restore(src, dst string) (err error) { | |||
|
|
|||
| written, err := r.a.Extract(dst, pr) | |||
| if err != nil { | |||
| err = fmt.Errorf("extract files from downloaded archive, pipe reader failed, %w", err) | |||
| err = fmt.Errorf("failure in extracting files from downloaded archive, %w", err) | |||
There was a problem hiding this comment.
failed to extract files from downloaded archive
There was a problem hiding this comment.
Does this print the archive name (and associated cache path)?
Aishwarya-Lad
force-pushed
the
CI-10788
branch
2 times, most recently
from
51b5ca7
to
f80525a
Compare
Modify error msg to warning msg modify error
| @@ -93,7 +93,7 @@ func (r rebuilder) Rebuild(srcs []string) error { | |||
| wg.Wait() | |||
|
|
|||
| if errs.Err() != nil { | |||
| return fmt.Errorf("rebuild failed, %w", errs) | |||
| level.Warn(r.logger).Log("msg", "ignore archival, ", errs.Err()) | |||
There was a problem hiding this comment.
The message is unclear IMO. When will the Err() be not nil? We should be checking for the case where the path directory doesn't exist and only then ignore it instead of ignoring it for all errors. Also let's print it clearly: path %s doesn't exist. Ignoring save to cache or similar.
There was a problem hiding this comment.
I think this should only just print warning when it's auto detected. This will change old behavior right?
| @@ -140,7 +140,7 @@ func (r rebuilder) rebuild(src, dst string) (err error) { | |||
| tr := io.TeeReader(pr, sw) | |||
|
|
|||
| if err := r.s.Put(dst, tr); err != nil { | |||
| err = fmt.Errorf("upload file, pipe reader failed, %w", err) | |||
| err = fmt.Errorf("failed to upload file to cache storage, %w", err) | |||
| if err := pr.CloseWithError(err); err != nil { | |||
| level.Error(r.logger).Log("msg", "pr close", "err", err) | |||
There was a problem hiding this comment.
Let's use a better message instead of "pr close"
Please read CONTRIBUTING.md for details on our code of conduct, and the process for submitting pull requests to us.
Fixes:
Appropriate error message when directory not found for restore/save.
Proposed Changes
Description
Test:
https://app.harness.io/ng/account/9UuUfLwaQ-6ZowvbG7qtLQ/home/orgs/default/projects/aishwaryawksp/pipelines/CI10788/executions/gfxDwpKyRCerkqoAm5QS8w/pipeline
Checklist
Unreleasedsection of CHANGELOG.