Skip to content
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

Implement error handler #105

Merged
merged 20 commits into from
Dec 18, 2024
Merged

Implement error handler #105

merged 20 commits into from
Dec 18, 2024

Conversation

MetalBlueberry
Copy link
Contributor

@MetalBlueberry MetalBlueberry commented Dec 17, 2024

This implements a flag for the CLI to ignore error while processing a file and enables an option to store the failures as a file.

When using as a package, it exposes a callback to handle errors that can be implemented by the user application.

The location info for error has been updated as well to include the Byte position. This allows for easy recovery as you know exactly where to start reading

Copy link
Contributor

@AdrianLC AdrianLC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see anything serious so LGTM 👍

Comment on lines +120 to +131
batchErrorHandler := csvcopy.BatchHandlerError()
if skipBatchErrors {
batchErrorHandler = csvcopy.BatchHandlerNoop()
}
if batchErrorOutputDir != "" {
log.Printf("batch errors will be stored at %s", batchErrorOutputDir)
batchErrorHandler = csvcopy.BatchHandlerSaveToFile(batchErrorOutputDir, batchErrorHandler)
}
if verbose || skipBatchErrors {
batchErrorHandler = csvcopy.BatchHandlerLog(logger, batchErrorHandler)
}
opts = append(opts, csvcopy.WithBatchErrorHandler(batchErrorHandler))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this could be extractor to an auxiliar function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far I think main is the best place to put this logic, moving this to a function won't make it much easier. as you need to pass all the flags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can have more functions directly in the main.go file so you would still have access to the flags since they're global. It's just a way to avoid an ever-growing main function that happens as programs become more complex

pkg/batch/scan.go Outdated Show resolved Hide resolved
pkg/batch/scan.go Outdated Show resolved Hide resolved
pkg/batch/scan.go Show resolved Hide resolved
pkg/batch/scan.go Outdated Show resolved Hide resolved
pkg/csvcopy/batch_error.go Show resolved Hide resolved
pkg/csvcopy/csvcopy_test.go Outdated Show resolved Hide resolved
pkg/csvcopy/csvcopy_test.go Outdated Show resolved Hide resolved
pkg/csvcopy/csvcopy_test.go Outdated Show resolved Hide resolved
pkg/csvcopy/csvcopy_test.go Show resolved Hide resolved
MetalBlueberry and others added 9 commits December 17, 2024 18:57
Co-authored-by: Adrián López Calvo <[email protected]>
Signed-off-by: Victor Perez <[email protected]>
Co-authored-by: Adrián López Calvo <[email protected]>
Signed-off-by: Victor Perez <[email protected]>
Co-authored-by: Adrián López Calvo <[email protected]>
Signed-off-by: Victor Perez <[email protected]>
…caledb-parallel-copy into vperez/skip-invalid-rows
@MetalBlueberry MetalBlueberry marked this pull request as ready for review December 17, 2024 18:34
@MetalBlueberry MetalBlueberry merged commit 7f3c49d into main Dec 18, 2024
3 checks passed
@MetalBlueberry MetalBlueberry deleted the vperez/skip-invalid-rows branch December 18, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants