Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Discussion: pip installable skill plugins #2958

Closed
forslund opened this issue Jul 24, 2021 · 19 comments
Closed

Discussion: pip installable skill plugins #2958

forslund opened this issue Jul 24, 2021 · 19 comments
Labels
Status: For discussion Feature proposal in development. Community input and discussion is invited. Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap.

Comments

@forslund
Copy link
Collaborator

forslund commented Jul 24, 2021

In the PR for storing skills in XDG data directory #2803 discussion around creating proper skill modules. I think the XDG PR is a good direction to take and shouldn't be hampered by this separate technical discussion, and the change is big enough to warrant it's own issue. So here it is!

There are a number of benefits from having the skills in plugins

  • proper version management
  • no need for extra infrastructure to support updates (possibly a json file to map mycroft-core version to skill module versions) but no msm or git-based tool to download / install skills
  • the skill creation knowledge is transferable to creating python modules, so a newbie skill creator can take what she or he learns into work / school etc
  • Better for system installation

The major downside (as pointed out by @ChanceNCounter) with doing the change is that more python knowledge is needed from the skill creator, to set up a proper module. In short it's not as simple and easy to grasp as the current system.

Things that needs to be investigated

  • How to best perform updates
  • How do we limit the version to only install versions compatible with a certain release of core
  • Should there be a separate pip-repo

Another big question is should the pip-installable skills replace the current system or be in addition to the current system. If it is in addition to the current system some of the benefits mentioned above won't happen but other advantages exists. For example skills implementing core functionality can be installed as plugins making sure an update of core is matched with the correct skill version. But community-created skills can still be done in the current relatively easy fashion.

Probably more.

The plugin system in core can easily be used to find and load skill modules. A minimal example for doing the loading can be seen in forslund@464931a and a compatible skill plugin is found here. This version treats the current msm installed skills and the plugin-loaded skills exactly the same and uses the same loading procedure.

@JarbasAl has created a similar plugin loader using a slightly different approach that can be found here: NeonJarbas/NeonCore@0735033 which creates a separate plugin loader class which can't be reloaded. As I understood his reasoning for that was that it was mainly for default skills that could be provided at a specific version in the requirements.txt / setup.py for neon/mycroft-core.

Would be great to hear more ideas and concerns around a system like this.

@ChanceNCounter
Copy link
Contributor

I think the status quo, where skills are just applets under a specified directory and the rest is Mycroft's problem, is ideal for newbies. I think tools are obstacles.

@NeonDaniel
Copy link
Member

I think the status quo, where skills are just applets under a specified directory and the rest is Mycroft's problem, is ideal for newbies. I think tools are obstacles.

I generally agree with the current method being most approachable.. I like the idea of skills being encapsulated modules for a number of reasons (mainly easier unit testing/dependency structure and version management), but this is less approachable as a new dev. I think both methods should be supported (skills directory + skills modules); for my dev workflow, I have a set of skills cloned that I test against different cores and I'm sure there are other use cases that should remain supported. For deployed systems, it would be nice to manage a base set of skills like any other python dependencies.

One issue I anticipate is that if skills can be installed 2 different ways, it seems likely that a user will have to address the same skill installed both ways (in the skill directory and as a module). I think it makes sense to just give one priority over the other and log a warning, but this could raise questions with settingsmeta and other data stored in shared locaitons..

@forslund
Copy link
Collaborator Author

I think if there is a switch to pip installable plugins there would likely be a time where the "old style" is supported before being removed completely.

  1. Support pip installable plugins until this is stable and working as we like
  2. Announce that pip installable plugins is the preferred way, mark current skill system deprecated and set a removal date
  3. Remove support for loading old skills

We can do a subtle change in the pip-installable skills to make them not loadable through the old load system so plugins can only be installed one way...

@NeonDaniel
Copy link
Member

I think if there is a switch to pip installable plugins there would likely be a time where the "old style" is supported before being removed completely.

  1. Support pip installable plugins until this is stable and working as we like
  2. Announce that pip installable plugins is the preferred way, mark current skill system deprecated and set a removal date
  3. Remove support for loading old skills

We can do a subtle change in the pip-installable skills to make them not loadable through the old load system so plugins can only be installed one way...

That makes sense if a changeover is desired, though I don't think support should be dropped for the old/current methodology. I suppose that editable pip installation would make it possible to have core try to install any pip-installable skills in the skills directory to prevent duplicates AND try to match the user's intent (assuming a cloned skill version takes priority over a pip installed one). I think this would mean that pip removes the version installed to the venv and updates the reference to the locally cloned skill (could be a config value to overwrite installed versions with locally cloned ones).

Maybe it would make sense to address packaged skills in the skill directory at the same time here, so a skill designed for pip installation can still be loaded from source on the local file system? This might address the issue @ChanceNCounter mentioned (at least in part) by letting a user just drop a skill in a directory and make changes in real-time.

@JarbasAl
Copy link
Contributor

in general i think skills should be designed for one mechanism or the other, and the mechanisms should coexist

some skills are core functionality (eg, stop) and could be handled by the package manager as part of the initial installation, i think this is a benefit. Being able to pin and revert skill versions in a reproducible way for each install is also a benefit, msm is not flexible enough

the issue here becomes the skill id, it comes from folder name, until now that was unique, now duplicates are possible

My approach linked above (WIP) is to assume a user skill is meant to override a system skill, in case of conflicting ids the system skill should not load/be unloaded as applicable

Example skill for implementation linked above https://github.com/NeonJarbas/about.neon ,

note that this would not be a valid skill if git cloned since __init__.py was moved, this was not purposeful but can be used to differentiate both approaches, enforcing that skills should always be installed a specific way

@NeonDaniel
Copy link
Member

NeonDaniel commented Jul 26, 2021

My approach linked above (WIP) is to assume a user skill is meant to override a system skill, in case of conflicting ids the system skill should not load/be unloaded as applicable

This is approximately what I was suggesting above. I agree that a user skill (git) should override system (pip) and additionally propose that if I clone the above about.neon, it should be loaded. My main concern is that it becomes more difficult to document/communicate how to modify a published skill that is designed to be pip installed. I see the transparency and simple real-time reloading of changes as an important feature (especially for new users).

@forslund
Copy link
Collaborator Author

Thanks @JarbasAI and @NeonDaniel, I updated the original post with the question if skill plugins should replace current skill system or if we should have it in addition to the current system.

Personally I'm a bit against having two ways of creating skills, makes things in core more complex (though as I suggest above there would be a transition time where it is more complex anyway) and I feel some of the gains from having the plugin system is lost if we don't use it 100%. Also it will likely be more confusing for a new skill developer seeing there is two different ways of making the skills.

Though having it only for system skills makes the implementation much easier handling different core versions etc will not be a problem.

@PureTryOut
Copy link
Contributor

You guys know why I want this but I'll add it for completions sake: pip installable skills means systems package manager installable skills. As a packager for a Linux distro (Alpine Linux as you all know) it would be great to be able to package skills up and keep track of them through the systems package manager rather than letting the software itself do it. This provides some nice system integration and means skills get the same quality assurance as the rest of your distribution. Together with pip installed skills on the user level this would means you can have the system administrator enforce some specific skills to be installed, and have the user choose it's own skills on top. Also this would allow some extra nice things like when user installs mycroft-plasmoid (or other non-skill thing related to Mycroft): install required skill for that too.

I'm personally not a fan of having two ways to create skills either. The argument that the current system is easier for "newbs" is imo not a good one, if you're not able to create a regular Python package then you will not have an easy time writing a proper skill either. It's not that hard and it will allow you to transfer your skills from writing Mycroft skills to regular Python modules and programs in the future. Why let the user get accustomed to some custom development method when Python has it's own thing built-in already?

How to best perform updates

Since Mycroft knows what skills are installed, just pip install -U <skill name> right? Or am I missing something obvious?

How do we limit the version to only install versions compatible with a certain release of core

I'm not sure how well non-installed Pip packages can be verified from a Python module, but maybe we can check before installation if the skill has some dependency version requirement set in setup.py like mycroft-core==20.08.0 or whatever, and refuse installation if that version mismatches the current running core?

Should there be a separate pip-repo

I don't see why. That is just more infrastructure to manage and the main Pip repo can be used perfectly fine for this use-case.

@forslund
Copy link
Collaborator Author

forslund commented Aug 3, 2021

If a mycroft-core dependency like that works that would be great and simple I will experiment with that in my test branch.

Edit: there is at least a way to block upgrades using a custom install command.

@ChanceNCounter
Copy link
Contributor

As a packager for a Linux distro (Alpine Linux as you all know) it would be great to be able to package skills up and keep track of them through the systems package manager rather than letting the software itself do it. This provides some nice system integration and means skills get the same quality assurance as the rest of your distribution.

I'm still not convinced packaging and tracking skills is any of the system's business, nor do I see why we should trust other people to package our stuff, perform QA, or keep their packages up to date. @PureTryOut is the exception, not the rule, on an internet cluttered with abandoned, third-party packages of X, Y and Z. I can't be the only non-distro-focused developer who finds the whole concept off-putting. I can ship code, I can ship binaries, I don't need other people putting their scripts on my code. It's not like users can tell the difference.

@NeonDaniel
Copy link
Member

I'm personally not a fan of having two ways to create skills either. The argument that the current system is easier for "newbs" is imo not a good one, if you're not able to create a regular Python package then you will not have an easy time writing a proper skill either.

I can only speak from my own experience getting started with Mycroft a couple years ago (for context this is also where I learned Python). The transparency of skill source made it way easier to troubleshoot and modify skills since it was easy to locate all the skill source in a directory and make changes that reloaded in real time.

The main drawback to skill modules I see is that it's non-trivial to set up a dev environment where you have all your pip-installed skills attached in an editable state. This could be handled in some utility or in the skill manager, but it does add complexity that new devs (esp. devs new to Python) might find off-putting.

@PureTryOut
Copy link
Contributor

The main drawback to skill modules I see is that it's non-trivial to set up a dev environment where you have all your pip-installed skills attached in an editable state.

That's quite easy to solve, just set your PYTHONPATH to the directory of the skill that's being developed and things will automagically work.

So all installed skills will go e.g. ~/.local/lib/python3.9/site-packages and you can have a PYTHONPATH="$HOME/mycroft-dev-skills which includes several locally developed skills and Python will figure it out automatically and load both paths.

@JarbasAl
Copy link
Contributor

JarbasAl commented Aug 10, 2021

pip install -e /path/to/skill

might need a restart depending if reload is implemented or not

@ChanceNCounter
Copy link
Contributor

The problem with PYTHONPATH for development is that now you're always in that dev environment.

This really seems like replacing something that works just because it's ugly. "Best" practices are not the only practices. Being noob-friendly and implementing "advanced" mechanisms for packaging are mutually exclusive concepts.

A related discussion came up at the OVOS board, and I'm gonna quote myself there, partially quoting myself here =P


For the sake of having commentary in the same place, I'm gonna repeat something I said in one of the other discussions: how a skill is distributed, and how users interact with the distribution channels, should boil down to how we conceive of a skill.

If skills are libs (or deps in general) they can, and perhaps should, be distributed and updated as such. That certainly applies to skills on which core depends, as well (probably) as platform-appropriate framework skills.

The majority of skills, though, are more like applets, for which the "app store" paradigm makes sense to devs and is familiar to users. It also makes it much easier to identify, as a user, the source and trust model of the software they're installing.

I think this could be a useful, loose measure for how to implement/classify something:

Is it intrinsic to your version of Core? --> Plugin
Is it meant for inheritance? --> Plugin
Is it now a plugin that also needs to be a skill? --> Publish as plugin
Is it still Not a Plugin? --> Skill


and the overwhelming majority of newb devs will be working in "applet" territory.

@krisgesling
Copy link
Contributor

Just wanted to throw an extra line of thought into the mix...

One of the big problems that I'd love to solve with "the next version of Mycroft Skills" is the ability to run them completely independently of mycroft-core. This would open up a bunch of potential for sandboxing Skills in different ways. They can be in their own venv, have different dependency version requirements, even use a modified python interpreter.

@NeonDaniel
Copy link
Member

Just wanted to throw an extra line of thought into the mix...

One of the big problems that I'd love to solve with "the next version of Mycroft Skills" is the ability to run them completely independently of mycroft-core. This would open up a bunch of potential for sandboxing Skills in different ways. They can be in their own venv, have different dependency version requirements, even use a modified python interpreter.

I agree with this 100%. Another factor is that this allows for much simpler unit testing; I think the first step here is being able to install the MycroftSkill dependency independent of a full core. An example of this is implemented here: https://github.com/NeonGeckoCom/neon-skill-utils/tree/master/neon_utils/skills (as used in Jarbas' above example skill).

@JarbasAl
Copy link
Contributor

i run skills externally from core via hivemind https://github.com/JarbasHiveMind/LocalHive/blob/master/local_hive/skill.py#L128

but i use HolmesV in order to import the mycroft skill class (without dragging all the other unused kruft, eg, dont need audio utils)

@JarbasAl
Copy link
Contributor

In ovos we do it like this OpenVoiceOS#9

@krisgesling krisgesling added Status: For discussion Feature proposal in development. Community input and discussion is invited. Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap. labels Nov 11, 2021
@forslund
Copy link
Collaborator Author

forslund commented Sep 8, 2024

Closing Issue since we're archiving the repo

@forslund forslund closed this as completed Sep 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: For discussion Feature proposal in development. Community input and discussion is invited. Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap.
Projects
None yet
Development

No branches or pull requests

6 participants