FRW-11561 duplications and deadlock fix#98
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
stereomon
left a comment
stereomon
left a comment
There was a problem hiding this comment.
While you are on it, can you check if PHPUnit update in this package?
|
|
||
| do { | ||
| $events = $this->getEventEntitiesByProcessId($processId, $offset, $limit); | ||
| static::$eventBehaviorTableExists = true; |
There was a problem hiding this comment.
this what a bit worries me. We set this property to true at some point. In the new implementation I can't see that. TBH I don't remember why we did this way, but this changes might break existing behaviour
| $this->triggerEventsAndDelete($events); | ||
| } while ($currentCountEvents === $limit); | ||
|
|
||
| if ($countEvents === $triggeredEvents && $triggeredEvents > 0) { |
There was a problem hiding this comment.
I guess this is here for some events that were not processed for some reason and we need to keep them in DB so the console command can pick the up later
| $primaryKeys = array_merge($primaryKeys, $this->getPrimaryKeys($events)); | ||
| $offset += $limit; | ||
| $this->triggerEventsAndDelete($events); | ||
| } while ($currentCountEvents === $limit); |
There was a problem hiding this comment.
removing limit adding offset pulling makes this loop not needed. But do we really need to remove the limit? It might be there to not process a lot of events at once.
And this is also makes the config value obsolete and this sounds a bit BC breaky to me
5 participants
PR Description
Add a meaningful description here that will let us know what you want to fix with this PR or what functionality you want to add.
Steps before you submit a PR
contribution-license-agreement.txtfile with the following content:I hereby agree to Spryker\'s Contribution License Agreement in https://github.com/spryker/event-behavior/blob/HASH_OF_COMMIT_YOU_ARE_BASING_YOUR_BRANCH_FROM_MASTER_BRANCH/CONTRIBUTING.md.This is a mandatory step to make sure you are aware of the license agreement and agree to it.
HASH_OF_COMMIT_YOU_ARE_BASING_YOUR_BRANCH_FROM_MASTER_BRANCHis a hash of the commit you are basing your branch from the master branch. You can take it from commits list of master branch before you submit a PR.Checklist