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

Named interface for Stop() #60

Open
kishaningithub opened this issue Jul 15, 2021 · 10 comments
Open

Named interface for Stop() #60

kishaningithub opened this issue Jul 15, 2021 · 10 comments

Comments

@kishaningithub
Copy link

Problem

Currently the signature of the Start function is

func Start(options ...func(*Profile)) interface {
	Stop()
} 

Use case

The lifecycle in our case is managed by another library (fluent-bit-go)

  • The Start() has to happen in lifecycle function FLBPluginInit
  • The Stop() has to happen in lifecycle function FLBPluginExit

Proposal

have a named interface for Stop

type Stopper interface {
	Stop()
} 

So the Start will become

func Start(options ...func(*Profile)) Stopper
kishaningithub added a commit to kishaningithub/profile that referenced this issue Jul 15, 2021
@davecheney
Copy link
Member

Thank you for raising this issue. Before we jump to solutions, can you explain why this is a problem?

@kishaningithub
Copy link
Author

Thanks for the quick response @davecheney

It is mainly because the Start() and Stop() has to be executed in 2 different places for fluent bit go lang plugins

To give a bit more context, Go plugin for fluent bit has the following lifecycle

Register -> Initialize -> Flush -> Exit

The flush gets called multiple times (~million times)

Initialize and exit gets called only once

I want the Start() to happen in initialize and Stop to happen in Exit()

Eg - https://github.com/fluent/fluent-bit-go/blob/master/examples/out_multiinstance/out.go#L21

Why is this a problem ?

I would like to assign the return value of the Start() to a variable and store it in a struct. During the Exit() i would access the value of this struct and call the Stop() method

Currently i am doing like the following

type PluginContext struct {
   ProfileStopper interface {
	Stop()
  } 
}

You can see a duplication of the Stop() interface in my code(the consumer of this library)

I would like to avoid this duplication and declare it just like this

type PluginContext struct {
   ProfileStopper profile.Stopper
}

@davecheney
Copy link
Member

As far as I understand this should work as the method sets of the anonymous and named interface as the same

https://play.golang.org/p/UVPn8HHZ-37

@kishaningithub
Copy link
Author

Thanks for the example @davecheney

The problem is not that it does not work, the problem is there is no named interface as far as i can see inside this library because of which i have to copy paste the same interface definition in my code

@davecheney
Copy link
Member

Could you show me what you’ve tried? Maybe we can fix it?

@kishaningithub
Copy link
Author

Currently the code i implemented is same as your example. There is no problem, it works

I felt it would be better if the anonymous interface has a name then i wont have to duplicate the interface definition as a consumer.

@davecheney
Copy link
Member

I felt it would be better if the anonymous interface has a name then i wont have to duplicate the interface definition as a consumer.

No, that's the wrong way to write Go. The caller should declare the interface it expects, don't import a package simply for its interface declaration.

@kishaningithub
Copy link
Author

That's a good thought @davecheney Thanks for sharing!

Just so that i understand clearly, Does this mean

  • Don't use interfaces exposed by libraries
  • Only use interfaces exposed by go lang (Eg, Reader, Writer, Error)

@kishaningithub
Copy link
Author

If you can point me to some articles/blogs that's fine too 😊

@kishaningithub
Copy link
Author

I still dont see this as a bad idea though. For example take this issue

golang/go#35306

Go lang wants to create a named interface for the Unwrap(), As(), Is() methods

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

No branches or pull requests

2 participants