feat: add fx options plugin #9010
Conversation
|
To elaborate on the plugin interface: the reason it takes the full list of fx options and returns the full list, is to maximize the ability to customize what gets passed to fx. Currently this isn't super useful because we pack everything into opaque
If this were flattened out instead of using In most cases, where you want to customize the implementation of an interface like |
|
Here's an example plugin that overrides the default Pinner with a custom one: func (p *PinnerPlugin) Options(opts []fx.Option) ([]fx.Option, error) {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
cfg, err := config.LoadDefaultConfig(context.Background())
if err != nil {
return nil, fmt.Errorf("loading AWS config: %w", err)
}
logger, err := zap.NewProduction()
if err != nil {
return nil, fmt.Errorf("constructing logger: %w", err)
}
smClient := secretsmanager.NewFromConfig(cfg)
pinsDB, err := db.NewPinsDB(ctx, logger.Sugar(), smClient)
if err != nil {
return nil, fmt.Errorf("constructing PinsDB: %w", err)
}
pinner := &pinner.Pinner{PinsDB: pinsDB}
return append(opts, fx.Replace(fx.Annotate(pinner, fx.As(new(pin.Pinner))))), nil
} |
|
2022-06-30 conversation:
|
|
@guseggert : I'm moving to the next iteration. Let me know if you disagree. |
guseggert
force-pushed
the
feat/fx-plugin
branch
3 times, most recently
from
3dea41b
to
cbef0cd
Compare
guseggert
force-pushed
the
feat/fx-plugin
branch
from
cbef0cd
to
3a5d9cd
Compare
This adds a plugin interface that lets the plugin modify the fx options that are passed to fx when the app is initialized. This means plugins can inject their own implementations of IPFS interfaces. This enables granular customization of go-ipfs behavior by plugins, such as: - Bitswap with custom filters (e.g. for CID blocking) Custom interface - implementations such as Pinner or DAGService - Dynamic configuration of libp2p ... One downside of this is that we're exposing the entire dependency graph, init hooks, initialization, etc. to users, so this comes with a caveat that we reserve the right to make breaking changes to the graph structure and initialization logic (although this historically happens rarely). If these things are changed, we should mention them in release notes and changelogs though, since they could impact users of this plugin interface. I'm not particularly fond of DI frameworks (and neither are some of the folks work on/near go-ipfs), but it seems unlikely that somebody will rewrite the dependency wiring and lifecycle hooks of go-ipfs, and add dynamic extension points, so this seems like a palatable compromise. There are also problems that we should clean up in how model the go-ipfs app in fx, such as: - We make extensive use of nested fx.Options, which fx itself discourages because it "limits the user's ability to customize their application". It should be easy to flatten these out into a single []fx.Option slice. - We pass around a list of opaque libp2p opts, which makes it hard to customize after-the-fact...we should consider naming each of these opts and providing them to fx as proper dependencies, so that they can be explicitly overridden. - We call fx.Invoke() in some places with anonymous functions. We should instead only pass exported functions to fx.Invoke(), so that they have exported names, which would make it easier to remove/augment the invocations that happen when the app is initialized. These aren't blocking issues, they just make it harder and more brittle to customize go-ipfs with this plugin.
guseggert
force-pushed
the
feat/fx-plugin
branch
from
3a5d9cd
to
d1a92fb
Compare
| @@ -113,7 +113,7 @@ require ( | |||
| go.uber.org/zap v1.21.0 | |||
| golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e | |||
| golang.org/x/sync v0.0.0-20210220032951-036812b2e83c | |||
| golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e | |||
| golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a | |||
There was a problem hiding this comment.
this bump doesn't seem necessary, but also seems highly unlikely to be a problem 😄
|
@guseggert I realized after merging that while this is fine we should add some docs about this to https://github.com/ipfs/kubo/blob/master/docs/plugins.md#plugin-types. Do you mind taking care of that in a follow up PR? |
This adds a plugin interface that lets the plugin modify the fx
options that are passed to fx when the app is initialized. This means
plugins can inject their own implementations of IPFS interfaces. This
enables granular customization of go-ipfs behavior by plugins, such
as:
Bitswap with custom filters (e.g. for CID blocking)
Custom interface implementations such as Pinner or DAGService
Dynamic configuration of libp2p ...
One downside of this is that we're exposing the entire dependency
graph, init hooks, initialization, etc. to users, so this comes with a
caveat that we reserve the right to make breaking changes to the graph
structure and initialization logic (although this historically happens
rarely). If these things are changed, we should mention them in
release notes and changelogs though, since they could impact users of
this plugin interface.
I'm not particularly fond of DI frameworks (and neither are some of
the folks work on/near go-ipfs), but it seems unlikely that somebody
will rewrite the dependency wiring and lifecycle hooks of go-ipfs, and
add dynamic extension points, so this seems like a palatable
compromise.
There are also problems that we should clean up in how model the
go-ipfs app in fx, such as:
We make extensive use of nested fx.Options, which fx itself
discourages because it "limits the user's ability to customize their
application". It should be easy to flatten these out into a single
[]fx.Option slice.
We pass around a list of opaque libp2p opts, which makes it hard to
customize after-the-fact...we should consider naming each of these
opts and providing them to fx as proper dependencies, so that they can
be explicitly overridden.
We call fx.Invoke() in some places with anonymous functions. We
should instead only pass exported functions to fx.Invoke(), so that
they have exported names, which would make it easier to remove/augment
the invocations that happen when the app is initialized.
These aren't blocking issues, they just make it harder and more
brittle to customize go-ipfs with this plugin.
Closes #7653