-
Notifications
You must be signed in to change notification settings - Fork 134
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
Refactor plugin handlers to allow composable plugins. #70
base: main
Are you sure you want to change the base?
Conversation
52c0c35
to
680f347
Compare
Also apparently I totally missed #68, though there are some differences in how we've structured our approach to the problem. |
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.
Can you add a usage example to README.md
like the implementation in #68? This should make it a little easier to compare
// HandleFunc registers a function to handle a request path with. | ||
func (h Handler) HandleFunc(path string, fn func(w http.ResponseWriter, r *http.Request)) { | ||
h.mux.HandleFunc(path, fn) | ||
} | ||
|
||
// AddImplementation adds the given implementation string to the manifest of the plugin handler. | ||
// Unique implmentation names are only added once. |
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.
s/implmentation/implementation/g
This patch refactors the plugin handlers so they implement http.Handler, and exposes the individual initMux methods as public methods. This allows an advanced user to combine multiple drivers and plugin handlers under a common mux - enabling the likely use-case of a network driver and IPAM driver which should be launched together. This patch is mostly backwards compatible with the existing API with the exception of sdk.Handler.NewHandler() which by necessity is adapted to provide for step-wise addition of implementations.
680f347
to
e740047
Compare
Usage is virtually identical (we took the same approach). However I was experimenting with this last night and found on docker 1.13 it looks like multi-serving is bugged (possibly on earlier versions too) - moby/moby#30792 |
I was wondering what the status of this PR is... since it's quite old and it apparently also has some merge conflicts by now. Also, the comment regarding the docker issue mentioned in #70 (comment) Reason I am asking is that I am currently trying to do exactly what this PR seems to provide: The ability to compose a plugin with multiple providers (IPAM and network). Is this going anywhere? That said, is there a different/more modern way to create Docker (network) plugins or is this SDK/repo still "the way"? |
This patch refactors the plugin handlers so they implement http.Handler, and
exposes the individual initMux methods as public methods. This allows an
advanced user to combine multiple drivers and plugin handlers under a common
mux - enabling the likely use-case of a network driver and IPAM driver which
should be launched together.
This patch is mostly backwards compatible with the existing API with the
exception of sdk.Handler.NewHandler() which by necessity is adapted to provide
for step-wise addition of implementations.
Closes #36.