Skip to content
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

Allow multiple annotations per function #899

Open
sywhang opened this issue Jul 20, 2022 · 3 comments
Open

Allow multiple annotations per function #899

sywhang opened this issue Jul 20, 2022 · 3 comments

Comments

@sywhang
Copy link
Contributor

sywhang commented Jul 20, 2022

With the new OnStart annotations being added in #895, some annotations actually might make sense for multiple annotations to take place for the same function.

For instance, the following nested annotation would allow us to specify OnStart hooks using annotations without being confusing about the order in which the hooks are executed:

fx.Annotate(
  fx.Annotate(
    func() { ... },
    fx.OnStart(...) // this hook will run first
  ),
  fx.OnStart(...) // followed by this
)
@vallahaye
Copy link

vallahaye commented May 4, 2024

Hello @sywhang,

Sorry to dig up an old issue, for me it's very relevant and I'd like to support the idea with an additional use case.

The way I write a service in Go is by defining an interface that declares the core "business" methods, then have a structure implement it.

Most of the time, this structure also has Init and Shutdown methods to initialize/uninitialize some resources (e.g. subscribing to a pubsub mechanism). These two methods are only relevant to the implementation and are therefore not present in the interface. This is a fairly common practice and is supported out of the box by Service Weaver, for example.

To date, the best way to do this with Fx (v1.21) is as follows:

func provideService(lc fx.Lifecycle) Service {
  s := NewService() // NewService returns a pointer to the structure type, not the interface, which allows us to access the Init and Shutdown methods.
  lc.Append(fx.StartStopHook(s.Init, s.Shutdown))
  return s
}

var Module = fx.Module("service", fx.Provide(provideService))

Or:

var Module = fx.Module("service",
  fx.Provide(
    fx.Annotate(NewService, fx.OnStart((*ServiceImpl).Init), fx.OnStop((*ServiceImpl).Shutdown)),
    func(s *ServiceImpl) Service { return s },
  ),
)

If we could annotate an annotated function, I think we could rewrite the above code as follows:

var _ Service = (*ServiceImpl)(nil) // Interface guard to ensure that the call to fx.As can be satisfied

var Module = fx.Module("service",
  fx.Provide(
    fx.Annotate(
      fx.Annotate(NewService, fx.OnStart((*ServiceImpl).Init), fx.OnStop((*ServiceImpl).Shutdown)),
      fx.As(new(Service)),
    ),
  ),
)

Also note that merging the two calls to Annotate (i.e. fx.Annotate(NewService, fx.OnStart((*ServiceImpl).Init), fx.OnStop((*ServiceImpl).Shutdown), fx.As(new(Service)))) doesn't work (rightly so) and produces the following error:

$ go run .
[Fx] PROVIDE	fx.Lifecycle <= go.uber.org/fx.New.func1()
[Fx] PROVIDE	fx.Shutdowner <= go.uber.org/fx.(*App).shutdowner-fm()
[Fx] PROVIDE	fx.DotGraph <= go.uber.org/fx.(*App).dotGraph-fm()
[Fx] PROVIDE	pkg.Service <= fx.Annotate(go.vallahaye.net/fxtest/pkg.NewService(), fx.As([[pkg.Service]]) from module "service"
[Fx] INVOKE		main.main.func1()
[Fx] ERROR		fx.Invoke(main.main.func1()) called from:
main.main
	/go/src/go.vallahaye.net/fxtest/main.go:9
runtime.main
	/usr/lib/go/src/runtime/proc.go:271
Failed: missing dependencies for function "main".main.func1
	/go/src/go.vallahaye.net/fxtest/main.go:9:
missing type:
	- *pkg.ServiceImpl (did you mean to use pkg.Service?)
[Fx] ERROR		Failed to start: missing dependencies for function "main".main.func1
	/go/src/go.vallahaye.net/fxtest/main.go:9:
missing type:
	- *pkg.ServiceImpl (did you mean to use pkg.Service?)
exit status 1

Let me know if you're still interested in solving this issue, I can take a closer look if necessary.

@vallahaye
Copy link

vallahaye commented May 20, 2024

Hello again @sywhang,

Although, in my opinion, the use case I described above is still valid, a new primitive that solves the problem (fx.Self #1201) has been implemented in the meantime.

@sywhang
Copy link
Contributor Author

sywhang commented May 20, 2024

hey @vallahaye, sorry for the delayed response - I haven't been too active on GitHub lately so apologies for the delay.

Yup, I am aware of fx.Self that was recently added. @JacobOaks who authored that PR works with me at Uber and we did look at your comment above during triaging, which inspired him to do that work :-) .

We'll be tagging a release shortly that releases that feature.

Meanwhile, allowing multiple annotations per function is still an issue we need to address and that may take a while more since the change involved is quite a bit more work.

HTH - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants