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

Convention over configuration #22

Open
palourde opened this issue Feb 5, 2020 · 3 comments
Open

Convention over configuration #22

palourde opened this issue Feb 5, 2020 · 3 comments

Comments

@palourde
Copy link
Contributor

palourde commented Feb 5, 2020

I believe we could decrease the number of decisions a plugin developer needs to do when using this SDK, by enforcing some conventions, and therefore simplify the process of creating your own plugin.

For example, the various NewGo... (e.g. sensu.NewGoHandler) require you to pass functions as arguments:

func main() {
	handler := sensu.NewGoHandler(&handler.PluginConfig, options, checkArgs, executeHandler)
	handler.Execute()
}

func checkArgs(_ *types.Event) error {}

func executeHandler(event *types.Event) error {}

I believe by adopting some conventions on the name of these functions, we could reduce the number of arguments required and therefore the complexity. For example, if we assumed that a Plugin must adhere to a given interface, we could simply do the following instead:

func main() {
	handler := sensu.NewGoHandler(&plugin)
	handler.Execute()
}

func (p *Plugin) Validate(_ *types.Event) error {}

func (p *Plugin) Execute(event *types.Event) error {}

Similarly, I don't believe declaring a PluginConfigOption should require that much attributes, e.g.

&sensu.PluginConfigOption{
			Path:      "example",
			Env:       "HANDLER_EXAMPLE",
			Argument:  "example",
			Shorthand: "e",
			Default:   "",
			Usage:     "An example configuration option",
			Value:     &handler.endpoint,
		},
	}

Instead, maybe we could simply assume sane default values (e.g. I assume the Path and the Argument could be similar) but allow those values to be overridden for edge cases.

@echlebek
Copy link
Contributor

echlebek commented Feb 5, 2020

+1, we could probably do much better with interfaces here.

@calebhailey
Copy link

Love it!

@jspaleta
Copy link
Contributor

One comment...
I don't think we always want to have an cmdline Argument backed by a path. The name of the path might be a convention we can set and have it match the argument name... but we will still need a bool to indicate if path override is desired.

Similarly with Env, we could enforce a naming convention but we'd still need a bool to enable/disable the optional Env binding.

Looking more closely... we actually could allow a PluginConfigOption with a Path but no cmdline argument set if we wanted to.

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

4 participants