koji

Clone

#892 cli: also load plugins from ~/.koji/plugins
Merged 4 years ago by mikem. Opened 6 years ago by julian8628.
julian8628/koji issue/887  into  master

file modified
+36 -16 
@@ -65,20 +65,40 @@ 
 

                  globals()[name] = v
 

  

  

- def load_plugins(options, path):
 

-     """Load plugins specified by our configuration plus system plugins. Order
 

-     is that system plugins are first, so they can be overridden by
 

-     user-specified ones with same name."""
 

+ def load_plugins(plugin_paths):
 

+     """Load plugins specified by input paths, ~/.koji/plugins, system plugins.
 

+     Loading order is descending, so they can be overridden by user-specified
 

+     ones.
 

+     Notice that:
 

+     - plugin file should end with .py extension
 

+     - non-directory is not acceptable by plugin_paths
 

+     - all plugin files and the exported handlers inside will be loaded, and
 

+       handler with the same name will override the one has already been loaded
 

+       before"""
 

+ 

      logger = logging.getLogger('koji.plugins')
 

-     if os.path.exists(path):
 

-         tracker = koji.plugin.PluginTracker(path=path)
 

-         for name in sorted(os.listdir(path)):
 

-             if not name.endswith('.py'):
 

-                 continue
 

-             name = name[:-3]
 

-             logger.info('Loading plugin: %s', name)
 

-             tracker.load(name)
 

-             register_plugin(tracker.get(name))
 

+     paths = []
 

+     # first, always load plugins from koji_cli_plugins module
 

+     paths.append(
 

+         '%s/lib/python%s.%s/site-packages/koji_cli_plugins' %
 

+         (sys.prefix, sys.version_info[0], sys.version_info[1]))
 

+     # second, always load plugins from ~/.koji/plugins
 

+     paths.append(os.path.expanduser('~/.koji/plugins'))
 

+     # finally, update plugin_paths to the list
 

+     if plugin_paths:
 

+         if not isinstance(plugin_paths, (list, tuple)):
 

+             plugin_paths = plugin_paths.split(':')
 

+         paths.extend([os.path.expanduser(p) for p in reversed(plugin_paths)])
 

+     tracker = koji.plugin.PluginTracker()
 

+     for path in paths:
 

+         if os.path.exists(path) and os.path.isdir(path):
 

+             for name in sorted(os.listdir(path)):
 

+                 fullname = os.path.join(path, name)
 

+                 if not (os.path.isfile(fullname) and name.endswith('.py')):
 

+                     continue
 

+                 name = name[:-3]
 

+                 logger.info('Loading plugin: %s', fullname)
 

+                 register_plugin(tracker.load(name, path=path, reload=True))
 

  

  

  def get_options():
 
@@ -124,6 +144,8 @@ 
 

      parser.add_option("--weburl", help=_("url of the Koji web interface"))
 

      parser.add_option("--topurl", help=_("url for Koji file access"))
 

      parser.add_option("--pkgurl", help=SUPPRESS_HELP)
 

+     parser.add_option("--plugin-paths", metavar='PATHS',
 

+             help=_("specify additional plugin paths (colon separated)"))
 

      parser.add_option("--help-commands", action="store_true", default=False, help=_("list commands"))
 

      (options, args) = parser.parse_args()
 

  
@@ -163,9 +185,7 @@ 
 

              else:
 

                  warn("Warning: The pkgurl option is obsolete, please use topurl instead")
 

  

-     plugins_path = '%s/lib/python%s.%s/site-packages/koji_cli_plugins' % \
 

-                    (sys.prefix, sys.version_info[0], sys.version_info[1])
 

-     load_plugins(options, plugins_path)
 

+     load_plugins(options.plugin_paths)
 

  

      if options.help_commands:
 

          list_commands()

file modified
+6 
@@ -37,6 +37,12 @@ 
 

  ;certificate of the CA that issued the HTTP server certificate
 

  ;serverca = ~/.koji/serverca.crt
 

  

+ ;plugin paths, separated by ':' as the same as the shell's PATH
 

+ ;koji_cli_plugins module and ~/.koji/plugins are always loaded in advance,
 

+ ;and then be overridden by this option
 

+ ;plugin_paths = ~/.koji/plugins
 

+ 

+ ;[not_implemented_yet]
 

  ;enabled plugins for CLI, runroot and save_failed_tree are available
 

  ;plugins =

file modified
+2 -1 
@@ -1699,7 +1699,8 @@ 
 

          'authtype': None,
 

          'debug': False,
 

          'debug_xmlrpc': False,
 

-         'pyver' : None,
 

+         'pyver': None,
 

+         'plugin_paths': None,
 

      }
 

  

      result = config_defaults.copy()

tests/test_cli/data/cli_plugins1/not_plugin.omg tests/test_cli/data/plugins/not_plugin.omg
file renamed
file was moved with no change to the file
tests/test_cli/data/cli_plugins1/plugin1.py tests/test_cli/data/plugins/plugin1.py
file renamed
+5 -1 
@@ -1,20 +1,24 @@ 
 

  from __future__ import absolute_import
 

  from koji.plugin import export_cli, export_as
 

  

+ 

  @export_as('foobar')
 

  @export_cli
 

  def foo():
 

      pass
 

  

+ 

  @export_cli
 

  def foo2():
 

      pass
 

  

+ 

  def foo3():
 

      pass
 

  

+ 

  foo4 = 'foo4'
 

  

+ 

  class bar():
 

      pass
 

-

tests/test_cli/data/cli_plugins1/plugin2.py tests/test_cli/data/plugins/plugin2.py
file renamed
file was moved with no change to the file
file added
+6 
@@ -0,0 +1,6 @@ 
 

+ from koji.plugin import export_cli
 

+ 

+ 

+ @export_cli
 

+ def foo5():
 

+     pass

file added
+7 
@@ -0,0 +1,7 @@ 
 

+ from koji.plugin import export_cli, export_as
 

+ 

+ 

+ @export_as('foo6')
 

+ @export_cli
 

+ def foo():
 

+     pass

file modified
+15 -5 
@@ -1,23 +1,33 @@ 
 

  from __future__ import absolute_import
 

- import mock
 

+ 

  import os
 

+ 

  try:
 

      import unittest2 as unittest
 

  except ImportError:
 

      import unittest
 

  

+ import mock
 

+ 

  from . import loadcli
 

+ 

  cli = loadcli.cli
 

  

  

  class TestLoadPlugins(unittest.TestCase):
 

- 

      @mock.patch('logging.getLogger')
 

-     def test_load_plugins(self, getLogger):
 

-         options = mock.MagicMock()
 

-         cli.load_plugins(options, os.path.dirname(__file__) + '/data/plugins')
 

+     @mock.patch('os.path.isdir')
 

+     def test_load_plugins(self, isdir, getLogger):
 

+         # skip system path and default user plugin directory check
 

+         isdir.side_effect = lambda path: False if path.startswith('/usr') \
 

+                             or path == os.path.expanduser("~/.koji/plugins") \
 

+                             else True
 

+         cli.load_plugins(os.path.dirname(__file__) + '/data/cli_plugins1:' +
 

+                          os.path.dirname(__file__) + '/data/cli_plugins2')
 

          self.assertTrue(callable(cli.foobar))
 

          self.assertTrue(callable(cli.foo2))
 

+         self.assertTrue(hasattr(cli, 'foo6'))
 

          self.assertFalse(hasattr(cli, 'foo3'))
 

          self.assertFalse(hasattr(cli, 'foo4'))
 

+         self.assertTrue(hasattr(cli, 'foo5'))
 

          self.assertFalse(hasattr(cli, 'sth'))

  • add plugin_path in koji.conf
  • add --plugin-paths in cli arguments

fixes: #887

I would swap the order, so system plugins are loaded first and possibly overridden by user ones.

I would swap the order, so system plugins are loaded first and possibly overridden by user ones.

User path will be searched in before system plugin path. Like the behavior of plugin2.py in unit test, if plugin filename is the same, the one in user path is loaded, and the one in system path is skipped, I think it should have the same result as the overriding approach.

if -p is not used, it fails on plugin_paths.append, as plugin_paths is None, so

else:
    plugin_paths = []

I've added one more comment, but yes, you're right and it works correctly for me.

rebased onto d92299c15d642b4d9d1ff70069d0bd03b95221e2
5 years ago

updated.
I added some extra info in help and comments in koji.conf

Dominance does not appear to be stable, at least in python3. If I define a plugin in ~/.koji/plugins that overrides the runroot command (but is not named runroot.py), then a random one appears to win.

Looks like we're trying to pick the first plugin file per name, which seems reasonable, but we then rely on imp.find_module to pick it from the set of paths.

rebased onto 43f9490b497cfdfcf15388c40772dd01924ab689
5 years ago

updated the loading order
will load plugins from module koji_cli_plugins -> ~/.koji/plugins/ > user defined dirs
and reload every file and method(no matter if the filename is duplicated)

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready
4 years ago

@julian8628 Hi, can you rebase this one? (merge conflict)

rebased onto f1b9d43
4 years ago

@tkopecek
Rebased

These are not appended, but prepended. This wording leads me to understanding, that user's plugin will always have priority, which is not true.

2 new commits added

  • adjust cli plugin config description
  • cli: also load plugins from ~/.koji/plugins
4 years ago

updated

2 new commits added

  • adjust cli plugin config description
  • cli: also load plugins from ~/.koji/plugins
4 years ago

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done
4 years ago

LGTM

This isn't quite where it needs to be, but I think further work can be deferred or at least split out.

Minor changes here. Please check:

https://github.com/mikem23/koji-playground/commits/pagure/pr/892

:thumbsup:

2 new commits added

  • shorten help text
  • fix whitespace
4 years ago

Thanks! @mikem
updated

Further work may be related to #1534

Commit 4e221c5 fixes this pull-request

Pull-Request has been merged by mikem
4 years ago
Changes Summary 9
+36 -16
file changed
cli/koji
+6 -0
file changed
cli/koji.conf
+2 -1
file changed
koji/__init__.py
+0 -0
file renamed
tests/test_cli/data/plugins/not_plugin.omg
tests/test_cli/data/cli_plugins1/not_plugin.omg
+5 -1
file renamed
tests/test_cli/data/plugins/plugin1.py
tests/test_cli/data/cli_plugins1/plugin1.py
+0 -0
file renamed
tests/test_cli/data/plugins/plugin2.py
tests/test_cli/data/cli_plugins1/plugin2.py
+6
file added
tests/test_cli/data/cli_plugins2/plugin2.py
+7
file added
tests/test_cli/data/cli_plugins2/plugin3.py
+15 -5
file changed
tests/test_cli/test_load_plugins.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.