Problem with container auto-group/one-per-scope types

[aaronc]

I have uncovered an issue with auto-group and one-per-scope types in the container API.

Basically, if you have a provider which provides auto-group or one-per-scope types, but these haven't been declared as auto-group or one-per-scope in the container yet we'll get an error. An example could be say we have an one-per-scope type upgrade.Handler. Many modules may provide this, but say we don't actually load the upgrade module which sets the OpenPerScope option on the container. Then all these other modules will fail. (By the way, there is no clean way for the upgrade module to even set the OnePerScope option if its Module is loaded from YAML)

I see two solutions. One use tags like dig, i.e.

type Outputs struct {
  container.Out

  UpgradeHandler upgrade.Handler `one-per-scope:"true"`
  QueryCommands []QueryCommand `auto-group:"true"`
}

This has the overhead of needing to use tags everywhere and if we don't, the proper one-per-scope or auto-group behavior isn't followed in this instance and then we get an error.

An alternative would be marking the types at compile time with some sort of embedded struct, i.e.

package upgrade

type Handler struct {
  container.OnePerScope

  Migrations []Migration
}

I think this is maybe cleaner because it puts the burden on framework developers rather than module developers. One downside is we couldn't use this with type aliases, i.e. instead of type QueryCommand *cobra.Command, we'd need a struct like:

type QueryCommand struct {
  container.AutoGroup

  *cobra.Command
}

Thoughts @jgimeno @fdymylja ?

[aaronc]

Another alternative is to just have public tag interfaces that get implemented by these special types:

type OnePerScopeType interface {
  IsOnePerScopeType()
}

This will allow structs or type aliases to be used without module developers needing to worry about a weird embedded struct field. I think I like this interface approach best.

[fdymylja]

Another alternative is to just have public tag interfaces that get implemented by these special types:

type OnePerScopeType interface {
  IsOnePerScopeType()
}

This will allow structs or type aliases to be used without module developers needing to worry about a weird embedded struct field. I think I like this interface approach best.

So if we wanted to provide a one-per-scope type we would need to impl this interface, same goes for auto-group.

It's unfortunate, but probably it is the best. It will need proper documentation because it's something that can be easily over-looked.

[aaronc]

So if we wanted to provide a one-per-scope type we would need to impl this interface, same goes for auto-group.

It's unfortunate, but probably it is the best. It will need proper documentation because it's something that can be easily over-looked.

These need to be configured anyway though. Seems more fool-proof than the container.Option which like I said can definitely be problematic.

So I think unless there's any objections I'll go with the interface approach. Does that sound fine?

[jgimeno]

Yeah @aaronc , I think is the best approach.

[aaronc]

Thanks for the input guys. Fixed in #9666 now.