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

Switch to registry system for installing features from extensions #601

Closed
adamcik opened this Issue Dec 5, 2013 · 6 comments

Comments

3 participants
@adamcik
Member

adamcik commented Dec 5, 2013

Motivating factor for this switch is that continuing to add more hooks for frontends, backends, gst-elements, library update providers, webclient dirs etc. looks like it will end up un-maintainable. We already have the library update provider hook which is essentially an extension specific hook, and will likely need to add another one for #440, web clients, and likely one for local playlists.

Additionally for cases such as having library and playlist providers for the local backend, solving things at an extension level feels a bit clunky. In particular the case that is bugging me is that say we want to have local-json provide library and playlists, we can only enable disable at an extension level. So say I want to keep the playlist provider as is, but change the library this is not possible. Of course one could have a local-json-library and a local-json-playlist to workaround this.

Ideally I would also like to have local/library=json and local/playlist=m3udir etc instead of doing extension toggling. This would also allow the default implementations to actually live inside the local while still being able to choose an alternate.

If we do go for some registry like system there would also be a very simple upgrade path assuming we have something like the following:

class Extension(object):
    def setup(self, registry):
        for backend in self.get_backend_classes():
            registry.register('backend', backend)

        ...

So essentially what I believe remains, if we decide to go this path is to figure out the exact implementation of the registry object, and how random extensions can use it to provide their own hooks.

On a final note, using setup tools entry points could also be possible, but having the single entry point is probably something we want to stick with.

@jodal

This comment has been minimized.

Member

jodal commented Dec 5, 2013

I think this sounds very reasonable and see no major drawbacks with this approach.

I agree that we probably want to use setuptools entry points just to find the Extension class, which then does the rest.

We'll of course have some standard hooks that matches the Extension methods we have today. For extension specific hooks, I suggest we dictate that they must start with the extension name, e.g. local:library for the existing library hook and http:client for telling the HTTP frontend about a client that needs to be served.

Example usage:

  1. pip install mopidy-local-sqlite => extension is installed and enabled by default,
  2. mopidy config --set local/library sqlite to switch to using the library provided by the extension.
  3. Mopidy is started. The extension registers a provider for the local:library hook. The local backend use the local/library value to find the wanted library provider among the potentially multiple ones registered with the local:library hook.
@adamcik

This comment has been minimized.

Member

adamcik commented Dec 5, 2013

We can also consider having the registry object have register_{frontend,backend,gstelement,..} where we have one for each of our main extension points, and then a final generic one for the extension extensions.

@jodal

This comment has been minimized.

Member

jodal commented Dec 5, 2013

Not needed, IMHO, as it doesn't add much convenience or safety.

@adamcik

This comment has been minimized.

Member

adamcik commented Dec 5, 2013

I've also glossed over that fact that we likely need to have the actual call be register(extension, name, *args, **kwargs) and I would probably go for giving the setup call just a register function with the extension already bound as a partial.

@ghost ghost assigned adamcik Dec 5, 2013

@adamcik

This comment has been minimized.

Member

adamcik commented Dec 5, 2013

Oh and final note for the local case, I'll likely create a new interface that covers library and updaters in a single class, meaning the default local library just delegates to this new class, and the scanner uses it directly.

@ZenithDK

This comment has been minimized.

Contributor

ZenithDK commented Dec 30, 2013

Will it be possible to enable and disable extensions on run-time with this change?

@jodal jodal closed this in #617 Jan 9, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment