Skip to content

Fix #schedule_eofed documented with a default FIFO fallback it does not have - #983

Open
coipond-writer[bot] wants to merge 1 commit into
masterfrom
fix/scheduling-api-eofed-no-fifo-fallback-44184
Open

Fix #schedule_eofed documented with a default FIFO fallback it does not have#983
coipond-writer[bot] wants to merge 1 commit into
masterfrom
fix/scheduling-api-eofed-no-fifo-fallback-44184

Conversation

@coipond-writer

Copy link
Copy Markdown
Contributor

What

`Pro/Consumer-Groups/Scheduling-API.md:142-146`'s `#schedule_eofed` table row said implementation is optional "as a default FIFO implementation is done," matching the phrasing used for `#schedule_revocation`/`#schedule_shutdown`/`#schedule_idle`/`#schedule_periodic`.

Why it's wrong

`pro/processing/schedulers/base.rb:126-129` aliases `schedule_revocation`/`schedule_shutdown`/`schedule_idle`/`schedule_periodic` to `schedule_fifo`, but never defines `on_schedule_eofed` -- only the concrete `Default` subclass adds that alias (`schedulers/default.rb:86`). `connection/listener.rb:421` calls `@scheduler.on_schedule_eofed` unconditionally.

A user who subclasses `Base` directly (the documented extension point) and skips `schedule_eofed` hits `NoMethodError` the first time eofed jobs arrive -- unlike every other job type in this table.

Fixed the `Required` column and description to reflect this asymmetry.

Fixes #44184 (Redmine).

…ot have

pro/processing/schedulers/base.rb:126-129 aliases schedule_revocation/
shutdown/idle/periodic to schedule_fifo, but never defines
on_schedule_eofed - only the concrete Default subclass adds that alias
(schedulers/default.rb:86). connection/listener.rb:421 calls
@scheduler.on_schedule_eofed unconditionally.

A user who subclasses Base directly and skips schedule_eofed hits
NoMethodError the first time eofed jobs arrive, unlike every other job
type in this table. Fixed the Required column and description to
reflect this asymmetry.

Fixes #44184.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

0 participants