refactor(adapters): chi-style middleware via WithAdapters#6
Closed
klaidliadon wants to merge 3 commits into
Closed
refactor(adapters): chi-style middleware via WithAdapters#6klaidliadon wants to merge 3 commits into
klaidliadon wants to merge 3 commits into
Conversation
Introduce runnable.RunFunc and runnable.Adapter (func(next RunFunc) RunFunc)
as the core extension point, and runnable.WithAdapters Option that
wraps the runnable's runFunc with the supplied adapters left-to-right
(first listed = outermost wrapper).
Reshape adapters.Draining and adapters.Ticker as config-only
constructors that return runnable.Adapter — work is supplied via
runnable.New, not via the adapter call. Composition becomes:
runnable.New(reconcile, runnable.WithAdapters(
adapters.Draining(10*time.Second),
adapters.Ticker(2*time.Second),
))
The adapters package now imports runnable for the Adapter type;
runnable does not import adapters. Stop and runnable lifecycle are
unchanged.
Add adapters.Recovering (with a single PanicHandler callback, collapsing the v0.1 RecoveryReporter / StackPrinter split) and adapters.Retry, and remove the runnable.WithRecoverer and runnable.WithRetry Options they replace. Drop Status.Restarts: the field counted WithRetry re-entries via the onStart coupling and has no clean way to surface now that retry lives outside core. A later release can reintroduce per-attempt observability via an explicit event channel. Update examples/main.go and examples/ticker-with-drain to use the new adapters via runnable.WithAdapters.
Update the Adapters and Migrating-from-v0.1 sections of README and the adapters package doc comment to show the runnable.WithAdapters Option shape. Add Recovering and Retry to the migration table and call out the Status.Restarts removal.
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reshapes the v0.2 adapters into chi-style middleware. Each adapter is now a config-only constructor that returns a
runnable.Adapter, andrunnable.WithAdapterswraps the runnable's runFunc with the supplied list (first-listed = outermost). Work stays positional onrunnable.New; adapters become decoration that composes with other Options.Closes the marino39 review item asking for a chi-style middleware factoring. Stacked on top of #4 — review against
feat/drain-and-ticker, notmaster.Commits
f7fa106— RunFunc/Adapter types and WithAdapters Option in core; Draining and Ticker reshaped asrunnable.Adapterconstructors.2432924— Recovering and Retry migrated to adapters;WithRecovererandWithRetryremoved;Status.Restartsdropped (it counted retry re-entries via an onStart side-channel that doesn't survive the migration).768f979— README and adapters doc comment rewritten for the WithAdapters shape.Shape
vs. v0.1:
Test plan
go test -race -count=1 ./...— passes (runnable6.1s,runnable/adapters2.8s).golangci-lint run ./...— clean.Notes
runnable.Adapterlives in the core package sorunnable.WithAdaptersdoesn't have to import theadapterssubpackage. Theadapterspackage depends onrunnable, not the other way around.Status.Restartsis gone, pending a proper event/observer hook in a later release. Documented in the migration section of the README.