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

Plugin support #104

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Plugin support #104

wants to merge 9 commits into from

Conversation

aligator
Copy link
Collaborator

@aligator aligator commented Jun 6, 2021

As discussed in #91 we may need plugins.

This is a first draft, which uses the GoPlug lib, which I created specifically for timetrace.
Both, this PR and GoPlug are in a very experimental and untested stage.
So feel free to leave suggestions or even other ideas to implement plugins.

Currently it is possible to add custom commands to timetrace and to print text to stdout.
Argument parsing has to be done in the plugin separately. (It should be possible to just use another cobra instance for that??)
(however don't know yet how to pass the help-output (e.g. timetrace help hello) to the plugin. Maybe with SetHelpFunc)

This should be enough to implement #91 as plugin. (except getting the latest record)

Note: not tested on windows yet...

Example

I added a basic example which adds a new timetrace subcommand timetrace hello
To run it, first build the plugin by running

go generate

Then just run

go run . hello some other arguments...

It will print out "Hello World" and the other passed arguments.
The hello command will even be visible in the timetrace help.

As GoPlug works through stdin / stdout, it should be possible to write plugins in any language. (haven't tried that yet)

@aligator
Copy link
Collaborator Author

@dominikbraun
I re implemented GoPlug using json rpc over stdin / stdout. This simplifies the communication logic a lot.
Updated this experimental timetrace version to use that. I also added now a query for LoadLatestRecord so the notify command would be fully implementable based on the proposal of @FelixTheodor

Note: we may have discovered another possiblity:
Call plugins the way I do now, but instead of rpc, just use timetrace as lib.
-> downside: the plugin binaries would all basically contain timetrace (may result in bigger binaries)
-> downside: it is not possible to write plugins in other languages but with the json-approach it would be possible (in theory)
-> downside: I think plugins which "provide" other data sources would not be possible as there communication is needed.

-> upside: much easier to maintain -> for rpc we need to basically create one rpc endpoint for each public method.

It would also be possible to use the lib-approach for subcommand-plugins and the rpc approach for datasource-plugins, where then the plugins would contain the rpc server and timetrace would query the data through that from the plugins.

@FelixTheodor
Copy link
Contributor

@aligator this seems to be really cool, I will take a closer look at it this weekend and see if I can adapt the notify command as a plugin via your library :)

@dominikbraun
Copy link
Owner

dominikbraun commented Jun 18, 2021

@FelixTheodor But don't build something too serious. @aligator found yet another way to solve the plugin communication, but due to the (over-?) complexity of these approaches we're currently thinking about making timetrace usable as a Go package with plugins being completely isolated standalone binaries using that package.

@aligator
Copy link
Collaborator Author

aligator commented Jun 20, 2021

@dominikbraun
I now experiment with code generation to generate the api out of normal functions to reduce the api-maintenance overhead:
https://github.com/aligator/goplug/tree/code-generation

These methods are basically also just callable as lib. So if someone wants to use that approach it works also with the same api-methods.

The idea is that the only requirement for API-functions is to be a 'Method' of 'something' and that the 'something' is created at startup so that it can be passed to GoPlug in the beginning. In our case we could even use the timetrace struct.
To mark such methods they have to be annotated with "//goplug:generate". And currently a requirement is that it returns at least an error (as last return type)

package actions

import (
	"fmt"
	"math/rand"
	"strconv"
	"time"
)

type App struct {
	isSeeded  bool
	lastHello int
}

//goplug:generate
func (a *App) GetRandomInt(n int) (int, error) {
	if !a.isSeeded {
		rand.Seed(time.Now().UnixNano())
		a.isSeeded = true
	}

	return rand.Intn(n), nil
}

//goplug:generate
func (a *App) PrintHello() error {
	fmt.Println("Hellooooooo", strconv.Itoa(a.lastHello))
	a.lastHello++
	return nil
}

This results currently in generated code like this:
https://github.com/aligator/goplug/blob/code-generation/example/host/gen/actions.go

A plugin calls these methods as any other method:

func main() {
	p := New()
	p.SetSubCommand("rand", func(args []string) error {
		if len(args) < 2 {
			return errors.New("rand: invalid arg count")
		}

		parsedInt, err := strconv.Atoi(args[1])
		if err != nil {
			return err
		}

		rand, err := p.GetRandomInt(parsedInt)
		if err != nil {
			return err
		}

		p.Print(fmt.Sprintf("Random result for input %v: \n%v", args[1], strconv.Itoa(rand)))

		return nil
	})

	p.Run()
}

@aligator
Copy link
Collaborator Author

aligator commented Jun 22, 2021

@dominikbraun @FelixTheodor Updated goplug to do the code generation. Also updated the example timetrace plugin to use that.

Now it is nothing more needed than the annotation to LoadLatestRecord to make it available to plugins. (some param / return types are not supported yet, such as slices and I am not sure yet how we will handle something like SaveRecords, which needs a pointer to a project. @dominikbraun does it have to be the exact reference to a project? Or do only the values count?).

@FelixTheodor feel free to try to implement the notify plugin. Should be easy to do. But don't expect the plugin support to be in the final state :-)
@dominikbraun we have to discuss about how to proceed further

Still not tested if it works at all on windows... ^^ Works :-)

@retronav
Copy link
Contributor

retronav commented Aug 5, 2021

What do you guys think of using WebAssembly (WASI, to be specific) for the plugins?

@aligator
Copy link
Collaborator Author

@obnoxiousnerd also an interesting idea, but I think in this case it is much more overhead than needed. If we support plugins I would prefer any more native way than another execution layer (webassembly).

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

Successfully merging this pull request may close these issues.

4 participants