-
Notifications
You must be signed in to change notification settings - Fork 294
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
Populate a map by combining name and group attributes #1036
Comments
Internal Ref: GO-1864 |
Some thoughts on the behaviour:
Before this proposed change, this will fail at runtime for at least 2 reasons as listed in the summary above. But if we implement the logic, what should the behaviour be? Should a dig output with both a name and a group be treated as special and only go in maps to maximise backward compatibility? Should we avoid allowing a name or group to be re-used individually? Example Behaviour 1 output (name+group, name, group all allowed - keys are decomposable)
Example Behaviour 2 output (failure at Invoke time: a dig key is now treated as name+group){}
Example Behaviour 2b (failure: a dig key is now treated as name+group, but both constituent components cannot be unique elsewhere in the container. Fail at Provide time). e.g.
Other considerations: Should this new mode be specifically and explicitly enabled via a dig Container or Scope Option? If so, at which level? Should this be exposed to Fx? If so, how? |
As per Dig issue: uber-go#380 In order to support Fx feature requests uber-go/fx#998 uber-go/fx#1036 We need to be able to drop the restriction, both in terms of options dig.Name and dig.Group and dig.Out struct annotations on `name` and `group` being mutually exclusive. In a future PR, this can then be exploited to populate value group maps where the 'name' tag becomes the key of a map[string][T]
As per Dig issue: uber-go#380 In order to support Fx feature requests uber-go/fx#998 uber-go/fx#1036 We need to be able to drop the restriction, both in terms of options dig.Name and dig.Group and dig.Out struct annotations on `name` and `group` being mutually exclusive. In a future PR, this can then be exploited to populate value group maps where the 'name' tag becomes the key of a map[string][T]
As per Dig issue: uber-go#380 In order to support Fx feature requests uber-go/fx#998 uber-go/fx#1036 We need to be able to drop the restriction, both in terms of options dig.Name and dig.Group and dig.Out struct annotations on `name` and `group` being mutually exclusive. In a future PR, this can then be exploited to populate value group maps where the 'name' tag becomes the key of a map[string][T]
As per Dig issue: uber-go#380 In order to support Fx feature requests uber-go/fx#998 uber-go/fx#1036 We need to be able to drop the restriction, both in terms of options dig.Name and dig.Group and dig.Out struct annotations on `name` and `group` being mutually exclusive. In a future PR, this can then be exploited to populate value group maps where the 'name' tag becomes the key of a map[string][T]
As part of uber-go#380 we allowed names and groups tags/options to co-exist to ultimately support Fx feature request uber-go/fx#998. We now intend to support Map value groups as per uber-go/fx#1036. We will do this in 2 steps. 1. This PR will begin tracking any names passed into value groups with out changing any external facing functionality. 2. a subsequent PR will exploit this structure to support Map value groups.
As per Dig issue: uber-go#380 In order to support Fx feature requests uber-go/fx#998 uber-go/fx#1036 We need to be able to drop the restriction, both in terms of options dig.Name and dig.Group and dig.Out struct annotations on `name` and `group` being mutually exclusive. In a future PR, this can then be exploited to populate value group maps where the 'name' tag becomes the key of a map[string][T]
As part of uber-go#380 we allowed names and groups tags/options to co-exist to ultimately support Fx feature request uber-go/fx#998. We now intend to support Map value groups as per uber-go/fx#1036. We will do this in 2 steps. 1. This PR will begin tracking any names passed into value groups with out changing any external facing functionality. 2. a subsequent PR will exploit this structure to support Map value groups.
As per Dig issue: uber-go#380 In order to support Fx feature requests uber-go/fx#998 uber-go/fx#1036 We need to be able to drop the restriction, both in terms of options dig.Name and dig.Group and dig.Out struct annotations on `name` and `group` being mutually exclusive. In a future PR, this can then be exploited to populate value group maps where the 'name' tag becomes the key of a map[string][T]
As part of uber-go#380 we allowed names and groups tags/options to co-exist to ultimately support Fx feature request uber-go/fx#998. We now intend to support Map value groups as per uber-go/fx#1036. We will do this in 2 steps. 1. This PR will begin tracking any names passed into value groups with out changing any external facing functionality. 2. a subsequent PR will exploit this structure to support Map value groups.
As per Dig issue: uber-go#380 In order to support Fx feature requests uber-go/fx#998 uber-go/fx#1036 We need to be able to drop the restriction, both in terms of options dig.Name and dig.Group and dig.Out struct annotations on `name` and `group` being mutually exclusive. In a future PR, this can then be exploited to populate value group maps where the 'name' tag becomes the key of a map[string][T]
As part of uber-go#380 we allowed names and groups tags/options to co-exist to ultimately support Fx feature request uber-go/fx#998. We now intend to support Map value groups as per uber-go/fx#1036. We will do this in 2 steps. 1. This PR will begin tracking any names passed into value groups with out changing any external facing functionality. 2. a subsequent PR will exploit this structure to support Map value groups.
As per Dig issue: uber-go#380 In order to support Fx feature requests uber-go/fx#998 uber-go/fx#1036 We need to be able to drop the restriction, both in terms of options dig.Name and dig.Group and dig.Out struct annotations on `name` and `group` being mutually exclusive. In a future PR, this can then be exploited to populate value group maps where the 'name' tag becomes the key of a map[string][T]
As part of uber-go#380 we allowed names and groups tags/options to co-exist to ultimately support Fx feature request uber-go/fx#998. We now intend to support Map value groups as per uber-go/fx#1036. We will do this in 2 steps. 1. This PR will begin tracking any names passed into value groups with out changing any external facing functionality. 2. a subsequent PR will exploit this structure to support Map value groups.
What is the current status of this? I also have the following scenario where this would be really helpful: package main
import (
"context"
"fmt"
"go.uber.org/fx"
)
type Database struct {
Name string
}
type InputParams struct {
fx.In
Db1 *Database `name:"db1"`
Db2 *Database `name:"db2"`
}
type InputHookParams struct {
fx.In
Connections []Database `group:"sql_connections"`
}
type Databases struct {
fx.Out
Db1 *Database `name:"db1" group:"sql_connections"`
Db2 *Database `name:"db2" group:"sql_connections"`
}
func SetupDatabases() (Databases, error) {
return Databases{
Db1: &Database{
Name: "user",
},
Db2: &Database{
Name: "cart",
},
}, nil
}
func RunExample(p InputParams) {
fmt.Println("1: " + p.Db1.Name)
fmt.Println("2: " + p.Db2.Name)
}
func RegisterOnStopHook(lc fx.Lifecycle, conns InputHookParams) {
lc.Append(fx.Hook{
OnStop: func(ctx context.Context) error {
for _, c := range conns.Connections {
fmt.Println("closing connection with " + c.Name + " ...")
}
return nil
},
})
}
func main() {
fx.New(
fx.Provide(SetupDatabases),
fx.Invoke(RegisterOnStopHook),
fx.Invoke(RunExample),
).Run()
} I want to inject multiple instances of Is there another way to achieve that? |
I think this #1096 issue is related to this |
I had this same issue on January and by the time after reading the docs and other issues, I thought that my use case was a bad implementation of Fx, now that I'm more experienced with the Framework I guess it wasn't. I was going to write an Issue regarding this problem when I found this that matches my exact use case, is there any update on this @jquirke? Is there a reason why the PR haven't been approved and merged yet? Did you get to a dead end? May I help with something? Thank you @jquirke for all the work you've done, it really was the exact thing I was looking for! |
No dead end, it's just there are a ton of corner cases to cover and test and it requires coordination with the Fx Layer as well For the problem I was trying to solve within Uber there were other workable options, and it's something I've just let slip down the priority list. Feel free to take what I've done so far and run with it; I may try and finish this given the demand for the feature both in public and inside Uber but unfortunately I can't make any promises. |
Currently FX supports injecting multiple different implementations of an interface type using groups
https://pkg.go.dev/go.uber.org/fx#hdr-Value_Groups
This allows me to Provide multiple implementations of an interface Foo provided they are annotated with group, and collect them
where a particular Foo can be provided as
However, the Foos I received back are not able to be identified by name.
Describe the solution you'd like
I would like to be able to populate a map of named foos, using the existing name annnotation:
where a particular Foo can be provided as
This allows me a powerful capability: To name and group a type together, and obtain it as an unordered slice, a single instance by name, or as a map of names to instances.
Describe alternatives you've considered
A trivial solution is to extend Foo to include a Name() method, and then simply organise this into a map[string]Foo.
But there are many cases where components need to be looked up by name according to dynamic configuration, that re-implementing this for each case seems inelegant and would be nice to delegate to the DI framework.
Is this a breaking change?
It is difficult to imagine a scenario where this would cause a breaking change here; but of course, there is also the curse of hindsight
The text was updated successfully, but these errors were encountered: