Plugin framework #51

eguzki · 2018-07-11T14:02:28Z

3scale Toolbox CLI is based on cri library for building command line tools.
Plugin system also uses cri to leverage easy to develop, flexible and extensible plugin system.

3scale Toolbox will load plugins installed in gems or $LOAD_PATH. Plugins are discovered via Gem::find_files, then loaded.
Plugins must be named ‘3scale_toolbox_plugin’ (.rb, .so, etc) and placed at the root of your gem’s #require_path.

Plugins may add commands to 3scale CLI or may add subcommands to any existing command.
Subcommands may be added to main commands or other subcommands as children.

Nothing better than few examples to illustrate .

Let's create a plugin to add a main simple hello world command.

$ cat lib/3scale_toolbox_plugin.rb
require '3scale_toolbox/cli'

module FooCommand
  extend ThreeScaleToolbox::Command

  def self.command
    Cri::Command.define do
      name        'foo'
      usage       'foo [options]'
      summary     '3scale foo'
      description '3scale foo command'
      flag :h, :help, 'show help for this command' do |_, cmd|
        puts cmd.help
        exit 0
      end
      run do |opts, args, _|
        puts "foo done"
      end
    end
  end
end
ThreeScaleToolbox::CLI.add_command(FooCommand)

$ RUBYOPT=-Ilib 3scale foo
Hello World

Few things worth to highlight.

Your module must be extended by ThreeScaleToolbox::Command module. It allows your command to be added to CLI command tree.
Must implement command module function and return instance of Cri::Command from cri
Add your command to 3scale CLI command tree by calling ThreeScaleToolbox::CLI.add_command

Your plugin help is also available using builtin help command

$ RUBYOPT=-Ilib 3scale help foo
NAME
    foo - foo command

USAGE
    3scale foo [options]

DESCRIPTION
    This command does a lot of stuff.

OPTIONS
    -h --help         show help for this command

OPTIONS FOR 3SCALE
    -v --version      Prints the version of this command

Let's create a plugin to add a simple hello world subcommand for the builtin copy command.

$ cat lib/3scale_toolbox_plugin.rb
require '3scale_toolbox/base_command'
require '3scale_toolbox/commands/copy_command'

module FooCommand
  extend ThreeScaleToolbox::Command

  def self.command
    Cri::Command.define do
      name        'foo'
      usage       'foo [options]'
      summary     '3scale copy foo'
      description '3scale copy foo subcommand'
      flag :h, :help, 'show help for this command' do |_, cmd|
        puts cmd.help
        exit 0
      end
      run do |opts, args, _|
        puts "foo done"
      end
    end
  end
end

ThreeScaleToolbox::Commands::CopyCommand.add_subcommand(FooCommand)

$ RUBYOPT=-Ilib 3scale copy foo
foo done

Few things worth to highlight.

Your module must be extended by ThreeScaleToolbox::Command module. It allows your command to be added to CLI command tree.
Must implement command module function and return instance of Cri::Command from cri
Add your subcommand to 3scale CLI command tree by calling parent command's module's add_subcommand method.

Checking copy command help, it can be verified the new subcommand foo is added.

$ RUBYOPT=-Ilib 3scale help copy
NAME
    copy - 3scale CLI copy

USAGE
    3scale copy <command> [options]

DESCRIPTION
    3scale CLI copy tools to manage your API from the terminal.

SUBCOMMANDS
    foo         3scale copy foo
    service     3scale CLI copy service

OPTIONS
    -h --help         show help for this command

OPTIONS FOR 3SCALE
    -v --version      Prints the version of this command

eguzki · 2018-07-11T14:02:48Z

closing #50 as deprecated

unleashed · 2018-07-12T13:32:57Z

before I go on reviewing I have a doubt: is there support for config files? Having to specify paths on command invocation sounds very unergonomic, so having a setting for that would be nice, plus when adding subcommands they could be installed in the right place

eguzki · 2018-07-12T13:39:59Z

Next step will be implementation of "remotes" feature. Like git remote, CLI will allow adding, listing, updating and deleting remotes. Something like:

3scale remote add origin https://my_provider_key@3scale-admin.eguzki-amp-cli-01.master.staging.3sca.net
3scale remote add destination https://my_provider_key@3scale-admin.eguzki-amp-cli-02.master.staging.3sca.net

Then

3scale copy service origin destination -t new_service service_id

CLI framework should provide necessary API for commands to use it easily

mikz

👍 I think it is great. Idiomatic ruby, minimal meta programming. Pretty obvious where everything points, so it is IDE friendly.

mikz · 2018-07-16T15:23:59Z

lib/3scale_toolbox/commands/copy_command/copy_service.rb

+          end
+        end
+
+        def self.run(opts, args)


Could (sub)commands be classes too? I guess so, right?

It does not affect the plugin system at all, but I think it would be nice as an example to actually instantiate the class with parameters and then run it.

Sure. Just change module FooCommand for class FooCommand and example should work. But instantiate the class is not going to work as it is know. I will think about it. Initially modules are used because it did not make sense to create "objects" of commands.

The "command" can still rely on class level .run, but that one would do .new(opts, args).call to actually instantiate the class and call it.
That will allow "commands" to have state per execution without messing global/class variables.

Something like this?

$ cat lib/3scale_toolbox_plugin.rb require '3scale_toolbox/cli' class FooCommand extend ThreeScaleToolbox::Command def self.command Cri::Command.define do name 'foo' usage 'foo [options]' summary '3scale copy foo' description '3scale copy foo subcommand' flag :h, :help, 'show help for this command' do |_, cmd| puts cmd.help exit 0 end run do |opts, args, _| FooCommand.new(opts, args).call end end end def initialize(opts, args) @opts = opts @args = args end def call puts "foo done" end end ThreeScaleToolbox::Commands::CopyCommand.add_subcommand(FooCommand)

Then running

$ RUBYOPT=-Ilib 3scale copy foo foo done

This example works perfectly

Yep. Something like that. 👍 🥇

I think we should "promote" this kind of interface instead of class level one, because it is harder to make mistakes (like instance level variables in class methods).

But that does not need to be concern of this PR.

mikz · 2018-07-16T15:24:30Z

lib/3scale_toolbox/commands/copy_command/copy_service.rb

+
+        def self.exit_with_message(message)
+          puts message
+          exit 1


We will need to find some abstraction for this later, so it does not exit running test suite.

unleashed

This looks ok, with some doubts about the plugin system that I think will be cleared up. However I think having a way to specify settings in a config file will be necessary.

unleashed · 2018-07-16T16:03:17Z

docs/plugins.md

+    -v --version      Prints the version of this command
+```
+
+Now, package your plugin as a [gem](https://guides.rubygems.org/make-your-own-gem/) and let us know about it.


I think this part is the most fragile. Gems have a flat namespace where you basically would need to name your plugin as 3scale_toolbox-myplugin to avoid polluting this global shared namespace. I'd advocate having the code installed in something such as /usr/share/3scale_toolbox/plugins (or .local/share...) or having a config file setting to specify it.

At the very least, here we should suggest using a specific prefix for the gem names, and possibly enforce it when looking up plugin code.

Update: I see we are using a specific path to look up plugins, but I guess that is still not enforcing things like the above, and it is not clear how to install such plugins from this doc.

That is a common practice in Ruby ecosystem. https://guides.rubygems.org/name-your-gem/

We just require gems to have 3scale_toolbox_plugin.rb to be automatically loaded by the 3scale executable.

I don't really see any need to enforce any rules. People are free to do whatever they want to do.
It is common practice to name extensions prefixed with the gem name. Also it is common practice to namespace all your code with the correct namespace according to your gem name.

unleashed · 2018-07-16T16:09:03Z

lib/3scale_toolbox/commands/copy_command/copy_service.rb

+          }.reject { |key, value| !value }
+        end
+
+        def self.copy_service(service_id, source, destination, system_name)


this method is very long and calls for refactoring into smaller parts

Agreed, but it is an improvement over existing one file solution :) This PR is supposed to show off the plugin system, not to reimplement those commands in a nice, easy to test way.

unleashed · 2018-07-16T16:10:00Z

lib/3scale_toolbox/commands/import_command/import_csv.rb

+require 'uri'
+require 'csv'
+require '3scale/api'
+require '3scale_toolbox/base_command'


shouldn't both 3scale/api and 3scale_toolbox/base_command be assumed to have been required by the command loader?

I'm definitely for not assuming anything. At some point you are going to test this in isolation and it will fire back.

eguzki · 2018-07-17T17:42:25Z

I guess all doubts have been resolved. Further issues can be discussed in coming PR's.

eguzki added 13 commits July 6, 2018 17:37

removed gem from managed source code

5a9ce20

plugin framework

7ee2591

plugin framework: cri dependency added

1be791a

.gitignore: add vendor path usually used by bundler

2d7e98b

plugin framework: copy_command imported

bfb8724

plugin framework: copy_command refactored as command

bcf04e5

plugin framework: import command imported

28a52e6

plugin framework: import_command refactored as command

e430778

plugin framework: update command imported

44a8bdc

plugin framework: update_command refactored as command

c751f29

plugin framework: help command

b674c05

plugin framework: builtin commands

446e7c3

plugin framework: doc about building plugins

19d0867

eguzki requested a review from mikz July 11, 2018 14:03

eguzki mentioned this pull request Jul 11, 2018

Framework to make it easy adding new commands #48

Closed

.travis.yml: update build script

92d6da5

eguzki requested a review from unleashed July 12, 2018 13:06

mikz approved these changes Jul 16, 2018

View reviewed changes

unleashed approved these changes Jul 16, 2018

View reviewed changes

eguzki merged commit 37f5529 into master Jul 17, 2018

eguzki deleted the plugin-framework-second-try branch July 17, 2018 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin framework #51

Plugin framework #51

eguzki commented Jul 11, 2018

eguzki commented Jul 11, 2018

unleashed commented Jul 12, 2018

eguzki commented Jul 12, 2018 •

edited

Loading

mikz left a comment

mikz Jul 16, 2018

eguzki Jul 17, 2018

mikz Jul 17, 2018

eguzki Jul 17, 2018

mikz Jul 17, 2018

mikz Jul 16, 2018

unleashed left a comment

unleashed Jul 16, 2018

mikz Jul 16, 2018

unleashed Jul 16, 2018

mikz Jul 16, 2018

unleashed Jul 16, 2018

mikz Jul 16, 2018

eguzki commented Jul 17, 2018

Plugin framework #51

Plugin framework #51

Conversation

eguzki commented Jul 11, 2018

eguzki commented Jul 11, 2018

unleashed commented Jul 12, 2018

eguzki commented Jul 12, 2018 • edited Loading

mikz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unleashed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eguzki commented Jul 17, 2018

eguzki commented Jul 12, 2018 •

edited

Loading