so plugins with no group are now root plugins? that's a little weird, why not set them to root until (or if) we'll handle that?

Task as a group confuses me, at least as long as task is not a plugin itself ... didn't you have some glorious idea some time ago to make it somehow like that and support sub-plugins more nicely too?

Task as a group confuses me, at least as long as task is not a plugin itself ... didn't you have some glorious idea some time ago to make it somehow like that and support sub-plugins more nicely too?

Yeah, as discussed in IRC, 'group' is a bit wrong word. We have been using these groups to define what interfaces a plugin implements. Renaming from 'group' does seem a good idea to make it more clear.

so plugins with no group are now root plugins? that's a little weird, why not set them to root until (or if) we'll handle that?

Yeah, this probably relates to 'group' being a bad term. 'interface' is probably better. A plugin with no groups would have no defined interfaces, which means essentially it wouldn't do anything. We don't have a root level plugin interface defined at the moment, so there is no way to make root level plugins with our official plugin registration system at the moment. That is certainly something that should be added though, and migrate our current root level plugins into the official framework instead of the ad-hoc stuff they are doing now.

Current consensus is to rename the 'groups' attribute to 'interfaces', which much more accurately describes how we are using them.

@paranoidi Agreed the 'groups' name was awkward when using it like we were, but I feel like with the rename it aligns very well with how we are using it. I guess since our interfaces are all informal, we should really have a docs page that defines what they actually are. There is also the small question of the 'builtin' and 'api_ver' registration attributes, which only apply to the 'task' interface. Not sure how much that matters, but it feels a bit weird that they are at root level of plugin registration but don't apply to all plugins being registered.

There is also the small question of the 'builtin' and 'api_ver' registration attributes, which only apply to the 'task' interface.

Oh wait, I guess api_ver could refer to the plugin registration interface as well, in which case we should probably bump it here. Not sure on that one, it's meant the task interface historically.

@gazpachoking Yeah, api_ver is fine, builtin is a bit task specific though.. maybe we can live with it for now?

As for increasing the api_ver .. do the api_ver3 draft while at it? We have some that declare 3 but it sure is not using it in any meaningful way and the current ambiguity is not ideal ...

Some backward compatibility with ver2 could be achieved (like use groups arg and give warning)

I would love to have the roles keyword added into api_ver3 as well, it would tell more directly what each plugin does.

So

plugin.register(ClassName, ...,, roles=[plugin.INPUT])
plugin.register(ClassName, ...,, roles=[plugin.ACCEPT, plugin.REJECT])

We have some that declare 3 but it sure is not using it in any meaningful way and the current ambiguity is not ideal ...

Hmm. We do? That's a bit curious, as we never actually made it. 😉

Yeah, api_ver is fine, builtin is a bit task specific though.. maybe we can live with it for now?

Yeah, I'm fine with leaving builtin for now.
api_ver was task specific before, but I guess it's okay to have it apply to both the task api as well as the plugin registration api, as I think 'task' is the only core interface (and obviously plugin registration is also core.) 'list', 'notifiers', 'urlrewriter' are all added by plugins.

I would love to have the roles keyword added into api_ver3 as well, it would tell more directly what each plugin does.

Hmm, this would be another thing that is only specific to 'task' interface plugins at root level? (Agreed that metadata would be nice though.) I keep wondering if some of this stuff might be better as class attributes in the plugin class rather than keywords to the register argument.

Another question: Most of these interfaces look at the 'schema' property of a plugin to get the configuration. This means that if a plugin wants to implement multiple interfaces, it must take the same config in all contexts. Is that an issue? Should we allow separate schemas like schema_task, schema_search? (for example, maybe some torrent site plugin wants to have an input plugin as well as a search plugin that have different schemas)

+1 for separating schema context, that's already confusing with movie list and subtitle list, as they both have schema values that are relevant only per interface

If we do allow the different schemas per interface, should we still fall back to just 'schema' if there isn't a specific one defined for that interface? Is making more class level properties like 'schema_<interface_name>' the nicest way to do the different schemas? i.e.:

class MyPlugin(object):
    schema_task = {schema for task interface}
    schema_search = {schema for search plugin interface}
    schema = {should any interface fall back to this schema when there isn't a specific one? (enables backwards compat)}

Very confusing if one plugin has multiple different schemas depending on where they are being used. I would rather encourage to write a baseclass that is inherited under different names and schemas.

Very confusing if one plugin has multiple different schemas depending on where they are being used. I would rather encourage to write a baseclass that is inherited under different names and schemas.

@paranoidi Yeah, I think I'm leaning that way too now. The only thing I don't like about that is that it might make some plugin names suckier, like if you want a 'mytorrentsite' plugin for multiple roles they'd need to be called mytorrentsite_input, mytorrentsite_search, mytorrentsite_urlrewrite etc. We don't really have a problem with that yet though, so maybe it's not something to worry about so much.

@paranoidi I reverted the plugins that were declaring api v3 now, and added backwards compatibility for plugins declaring 'groups' with a deprecation warning. This means api_ver is still only being used by the task interface, and I think we can separate considering what api v3 should be, and whether we need to reconsider how api_ver works from this PR.
I also decided we don't need the schema contexts at the moment, so that can be considered separately later if we find a need.