-
Notifications
You must be signed in to change notification settings - Fork 481
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
Create an interface for the executors that can log errors, so that we can call it from the exporters #1314
Create an interface for the executors that can log errors, so that we can call it from the exporters #1314
Conversation
…able to call this anywhere without knowing if retrying is enabled
*/ | ||
abstract class InMemoryExceptionLogger { | ||
private Map<String, ErrorDetail> errors; | ||
private Map<String, ErrorDetail> recentErrors; |
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.
both seem to be the same. Unclear why we have 2 fields?
private Map<String, ErrorDetail> recentErrors; | ||
|
||
private void logError(String idempotentId, ErrorDetail errorDetail) { | ||
this.errors.put(idempotentId, errorDetail); |
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.
no need for "this."
@@ -34,7 +34,7 @@ | |||
import java.util.concurrent.Callable; | |||
|
|||
/** A {@link IdempotentImportExecutor} that stores known values in memory. */ | |||
public class InMemoryIdempotentImportExecutor implements IdempotentImportExecutor { | |||
public class InMemoryIdempotentImportExecutor extends InMemoryExceptionLogger implements IdempotentImportExecutor { | |||
private final Map<String, Serializable> knownValues = new HashMap<>(); | |||
private final Map<String, ErrorDetail> errors = new HashMap<>(); | |||
private final Map<String, ErrorDetail> recentErrors = new HashMap<>(); |
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.
I am not sure this is right. It looks like the executor has 2 sets or errors and adds / removes from them. In your new class, you have your own set. And always add to both. I feel it is a bug?
No description provided.