Skip to content
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

Openvariant | Add path to external plugin folder in annotation #21

Closed
FedericaBrando opened this issue Feb 27, 2024 · 7 comments · Fixed by #23
Closed

Openvariant | Add path to external plugin folder in annotation #21

FedericaBrando opened this issue Feb 27, 2024 · 7 comments · Fixed by #23
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@FedericaBrando
Copy link
Member

Openvariant iterates through the base folder, where it is run, trying to match a glob patter that looks like this: (os.getcwd()/**/[plugin]) to find the script for the plugin. See lines below:

try:
files = list(glob.iglob(f"{os.getcwd()}/**/{x[AnnotationTypes.PLUGIN.value]}", recursive=True))
if len(files) == 0:

This can give rise to two big problems:

  1. If you are running in a folder where there a lot of subfolders, the glob pattern will iterate through the whole working directory to find the script for the plugin. Decreasing speed performance.
  2. If you develop a package that uses openvariant and you want to have custom plugins it won't be found unless you run from the package folder.

a workaround for us (since we also develop openvariant) is just to include as many plugins in openvariant as we need and then update it on pip.

A better solution would be to include in the annotation file (annotation.yaml) a parameter for plugins that explicitly tells where to find the them. Although this can still lead to other caveats.

Since having the ability to use external plugins is a super nice feature, it would be nice to have a clean solution for developing specific packages that uses openvariant.

@FedericaBrando FedericaBrando added bug Something isn't working enhancement New feature or request labels Feb 27, 2024
@FedericaBrando FedericaBrando self-assigned this Feb 27, 2024
@FedericaBrando
Copy link
Member Author

FedericaBrando commented Feb 29, 2024

Possible solutions:

  • add an environment variable OPENVAR_PLUGIN that points to the folder where openvar needs to find the plugins
  • add a parameter to openvariant that takes in input plugin path.

@CarlosLopezElorduy
Copy link
Member

A possible use case I'm thinking of now is that a user might have a plugin in one folder (for example, one that he created) and another in a different folder (maybe one that another user created in a random folder, and for any reason cannot be moved). For this case, it might make more sense to use the second option (adding a parameter), since you would be able to add more than one folder.

I don't know if this use case is very specific and if it might happen or not in the first place, but it might be worth it to consider at the very least.

@dmartmillan
Copy link
Collaborator

I think it's good to consider this change.
I will get the second option, define the parameter for each specific plugin to point where the plugin is located.
Should the plugin be searched in the os.getcwd()/**/[plugin] path if the user doesn't add any plugin path (default case)? Or should this parameter be mandatory?

@CarlosLopezElorduy
Copy link
Member

I feel it would be cleaner to make it mandatory, for the cases that the user is using a plugin that is not already in OpenVariant.

@FedericaBrando
Copy link
Member Author

FedericaBrando commented Mar 6, 2024

Given yesterday discussion, we thought that a more functional way to proceed would be adding a default environment varianble exported at installation that stores automatically a path where plugins would be install.

i.e.

OPENVAR_PLUGIN = $HOME/.local/share/openvariant/plugins

In this way we can then overwrite the default environment variable for development purposes.

@migrau
Copy link
Member

migrau commented Mar 15, 2024

The PR looks good. Just remember to upload the documentation https://openvariant.readthedocs.io/en/latest/user_guide/plugin_system.html#plugin-system

@FedericaBrando
Copy link
Member Author

added documentation:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
4 participants