-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: silent error during batch ingestion #794
fix: silent error during batch ingestion #794
Conversation
@@ -387,7 +387,7 @@ func (usecase *IngestionUseCase) processUploadLog(ctx context.Context, uploadLog | |||
out := usecase.readFileIngestObjects(ctx, file.FileName, file.ReadCloser) | |||
if out.err != nil { | |||
setToFailed(out.numRowsIngested) | |||
return err | |||
return out.err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main point of the PR is here. The rest is mainly cleaning up potentially duplicate error handling
@@ -13,14 +12,6 @@ func LogAndReportSentryError(ctx context.Context, err error) { | |||
logger := LoggerFromContext(ctx) | |||
logger.ErrorContext(ctx, fmt.Sprintf("%+v", err)) | |||
|
|||
// Known issue where Cloud Run will sometimes fail to create the unix socket to connect to CloudSQL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be so much of an issue any more, this was a pain point when we were starting a cloud run job 3 times per minute, and just hitting the CloudSQL availability SLAs.
394eccb
to
98dfabd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.