-
Notifications
You must be signed in to change notification settings - Fork 80
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
Outbox: Improve batch handling #1286
base: trunk
Are you sure you want to change the base?
Conversation
includes/collection/class-outbox.php
Outdated
); | ||
|
||
if ( $existing_items ) { | ||
foreach ( $existing_items as $existing_item_id ) { |
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 think get_posts()
always returns an array, even if it's empty. If that's the case the if check shouldn't be necessary.
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.
At this point we should probably also look for any in-progress jobs and remove them, and also delete the outbox_offset post meta.
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.
how can I kill in-progress jobs?
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 guess we can't kill in-progress batches, but if this happens to fall in-between two batches we can remove the scheduled job. This is how we can check if there's a timestamp and then pass it to wp_unschedule_event()
.
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.
Do we need the actor ID in the schedule? this is part of the outbox item already!
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.
We don't, the only reason I kept it in was that we already had it in process_outbox
and I didn't need to get it again with self::get_author()
. Feel free to change that
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.
😩
* add some actions * fix phpdoc * Use "Unreleased" instead of version number * Add readme * Switch actions props @obenland * renamed action names * fix phpdoc * fix phpdoc * fix phpdoc * fix namings * fix phpcs issues
We'll do that in a separate PR
* Stream: Only surface errors in Outbox processing Also adds support for comment and user types. * Add changelog * fix readme * update to new batch processing * fix phpcs * fix phpcs * re-use wordings from the rest controllers * fix phpcs * restructure the output to match the errors * revert latest changes * Fixed changelog --------- Co-authored-by: Matthias Pfefferle <[email protected]>
Makes it a bit easier to see what's happening when reading the code.
* Outbox Batch: Only pass outbox id to jobs * Remove unnecessary imports
This allows us to filter by ID and to better debug by title.
f4ab80b
to
39a3a1c
Compare
After a new item is added to the Outbox, every pending item of the same type will be "invalidated". This way, we ensure that older
Update
s won't be published if there is already a newer one.Aside from that, this PR allows us to filter by ID and to better debug by title.