Feat/159/autodeposit #179
Feat/159/autodeposit #179
Conversation
|
Failing ci is likely unrelated to this branch and should be fixed by #181 |
Mikelle
force-pushed
the
feat/159/autodeposit
branch
from
b96e0e1 to
7236481
Compare
| } | ||
|
|
||
| func (adt *AutoDepositTracker) DoAutoMoveToAnotherWindow(ctx context.Context, ads []*bidderapiv1.AutoDeposit) error { | ||
| adt.isWorking.Store(true) |
There was a problem hiding this comment.
This should check first if it is already working before starting a new one right?
Probably need to use something like CAS instead of Store.
| return fmt.Errorf("error subscribing to event: %w", err) | ||
| } | ||
|
|
||
| eg.Go(func() error { |
There was a problem hiding this comment.
This separate goroutine is not needed. We can move this functionality to the goroutine below itself.
| } | ||
|
|
||
| func (adt *AutoDepositTracker) Stop() (*bidderapiv1.CancelAutoDepositResponse, error) { | ||
| if !adt.isWorking.Load() { |
There was a problem hiding this comment.
I think its better to have mutex here. We should not allow Stop to go through before the autodeposit is properly started. This flag doesnt really mean it was started properly as it is set in the start of the function.
There could be a race condition here where the cancelFunc is not set and stop returns deposits to cancel and says it has stopped the autodeposit process when in reality it hasnt.
Mikelle
force-pushed
the
feat/159/autodeposit
branch
from
7236481 to
c49dda6
Compare
Closing those issues:
#159
#154