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

Do not cache files that were part of a batch that had errors when formatting #449

Closed
brianmcgee opened this issue Oct 14, 2024 · 0 comments · Fixed by #450
Closed

Do not cache files that were part of a batch that had errors when formatting #449

brianmcgee opened this issue Oct 14, 2024 · 0 comments · Fixed by #450
Assignees
Labels
bug Something isn't working

Comments

@brianmcgee
Copy link
Member

Recent changes on main removed this logic:

// Append to batch and process if we have enough.
// We do not cache any files that were part of a pipeline in which one or more formatters failed.
// This is to ensure those files are re-processed in later invocations after the user has potentially
// resolved the issue, e.g. fixed a config problem.
if len(task.Errors) == 0 {
	batch = append(batch, task)
	if len(batch) == BatchSize {
		if err := processBatch(); err != nil {
			return err
		}
	}
}
@brianmcgee brianmcgee added the bug Something isn't working label Oct 14, 2024
@brianmcgee brianmcgee self-assigned this Oct 14, 2024
brianmcgee added a commit that referenced this issue Oct 14, 2024
This was a regression introduced when the new `walk.Reader` approach was applied.

When a formatter errors out during processing, we should not update those files in the cache to ensure they are retried during later invocations.

I've extended the `walk.ReleaseFunc` to accept an optional `error` which can be used to indicate the file participated in a batch that had errors when being formatted.

This allows the hook implementation to decide what it should do.

Closes #449

Signed-off-by: Brian McGee <[email protected]>
brianmcgee added a commit that referenced this issue Oct 14, 2024
This was a regression introduced when the new `walk.Reader` approach was applied.

When a formatter errors out during processing, we should not update those files in the cache to ensure they are retried during later invocations.

I've extended the `walk.ReleaseFunc` to accept an optional `error` which can be used to indicate the file participated in a batch that had errors when being formatted.

This allows the hook implementation to decide what it should do.

Closes #449

Signed-off-by: Brian McGee <[email protected]>
brianmcgee added a commit that referenced this issue Oct 14, 2024
This was a regression introduced when the new `walk.Reader` approach was applied.

When a formatter errors out during processing, we should not update those files in the cache to ensure they are retried during later invocations.

I've extended the `walk.ReleaseFunc` to accept an optional `error` which can be used to indicate the file participated in a batch that had errors when being formatted.

This allows the hook implementation to decide what it should do.

Closes #449

Signed-off-by: Brian McGee <[email protected]>
brianmcgee added a commit that referenced this issue Oct 14, 2024
This was a regression introduced when the new `walk.Reader` approach was applied.

When a formatter errors out during processing, we should not update those files in the cache to ensure they are retried during later invocations.

I've extended the `walk.ReleaseFunc` to accept an optional `error` which can be used to indicate the file participated in a batch that had errors when being formatted.

This allows the hook implementation to decide what it should do.

Closes #449

Signed-off-by: Brian McGee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant