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

support the creation fo an actual plugin dependency tree and use its root for consideration/order #51

Open
RonnyPfannschmidt opened this issue Mar 28, 2017 · 21 comments

Comments

@RonnyPfannschmidt
Copy link
Member

in pytest when we use conftests with pytest_plugins,
those specifications are not disabled when using other paths

as such, as soon as trees of conftests that use pytest_plugins, we get a inconsistent dependency state of plugins

additionally some plugins dod epend on other plugins either optionally or directly

experiments and a larger discussion are needed to give this issue the required detail for concrete action

@goodboy
Copy link
Contributor

goodboy commented Apr 20, 2017

@RonnyPfannschmidt wait doesn't #52 provide a way to address this?

We can temporarily register contest.py plugins for each path - de-registering as each new path is used?

@RonnyPfannschmidt
Copy link
Member Author

@tgoodlet #52 is absolutely dependent on this actually

after all temporary registrations imply sub-registrations of dependent plugins, and a context-manager is also not always desirable (in particular when plugins that add hooks come into play)

basically managing dependencies and dependent plugins is required in order to make something like the context manager sustainable

@goodboy
Copy link
Contributor

goodboy commented Apr 22, 2017

as such, as soon as trees of conftests that use pytest_plugins, we get a inconsistent dependency state of plugins

@RonnyPfannschmidt do we have an example of this being a problem?
A bug in pytest maybe or some code as an example?
Is pytest-dev/pytest#1821 somewhat related?

additionally some plugins do depend on other plugins either optionally or directly

@RonnyPfannschmidt I was thinking about this a tiny bit.
The package management system really should take care of this?
Isn't it the plugin writers responsibility to ensure plugin dependencies are met (even if relying on other plugins)?
I agree that with the case of conftest.py files using pytest_plugins this isn't as explicit and works outside the pkg manager.

I guess I'm looking for a deeper explanation of the problem ;)

@RonnyPfannschmidt
Copy link
Member Author

@tgoodlet the graph of plugin dependencies cant be drawn with the metadata python packaging provides atm

even more so if there is implicit and/or indirect dependence, currently we simply have no way to express the metadata needed to do it

@goodboy
Copy link
Contributor

goodboy commented Jul 11, 2017

@RonnyPfannschmidt If you can give me an example of this problem it'd be super helpul ;)

we get a inconsistent dependency state of plugins

Mostly I just need to have an idea of what ^ consists of.

@RonnyPfannschmidt
Copy link
Member Author

@tgoodlet any conftest that puts plugins into pytest_plugin enables them globally instead of for that folder tree

@pombredanne
Copy link
Contributor

Hi: any progress on this?

@nicoddemus
Copy link
Member

@pombredanne AFAIK nobody has taken the time to work on this, I'm afraid.

@pombredanne
Copy link
Contributor

ack, thanks. I would need something like this, so I may hack a few things that may be of reuse. TBD!

@goodboy
Copy link
Contributor

goodboy commented Jan 12, 2018

We always enjoy the help @pombredanne!

@pombredanne
Copy link
Contributor

@tgoodlet I have something working mostly on my side that I will push in a branch of scancode-toolkit soon.

But in doing I found out that I may use only a few capabilities of pluggy. In particular I do not use pluggy to run the plugins, only as a registry. And my plugins are using Click for the CLI "UI".

The way dependencies are expressed is on other plugins is that each plugin holds a list of project_name: plugin_name strings where plugin_name is the setuptools entrypoint name. This is only an attribute of my plugins, not pluggy. Based onthis and the actual execution context of user-selected options:

  1. I can check if a plugin is enabled (plugin-specific, typically based on its user-selected options or else)
  2. If yes, I can check if all its dependencies are also enabled, recursively, by walking the dependencies graph, detecting and erroring on cycles, and checking if each plugin is enabled.
  3. If yes, the plugin can run, otherwise an error is raised as dependencies are not met.

So all in all, it may be of little value for pluggy and pytest. And frankly I have no idea on how dependencies are handled by pluggy, though I would be glad to use this.

Anyway, I will ping you here when I push this.

@goodboy
Copy link
Contributor

goodboy commented Jan 15, 2018

@pombredanne sounds good.
I'm sure it will at the least provide ideas for what @RonnyPfannschmidt desires here.

@pombredanne
Copy link
Contributor

pombredanne commented Jan 27, 2018

So I am making some slow progress aboutcode-org/scancode-toolkit#885 in a branch. I have now --with great shame-- realized that I am only using pluggy as a glorified setuptools entrypoint loader and do not use any of the hooking execution mechanisms.

My plugins are classes and not functions :|

Nevertheless I need to resolve deps between plugins and each of my plugins will need some try fist and try last and this with deps creates some graph.
For now I crudely do it at the UI level using an enhanced Click option class that can express dep requirements on other options for a plugin to be "enabled". I will need to build a better graph that's not UI dependent instead.

So here is my question 1.

  • In evolving this mess o mine, could I use pluggy hooks to use classes instead? (I guess pluggy could work with any callable and not just functions. I need some state in each of my plugins, at least class-level state on a given "plugin")

And 2:

  • If I can move there, the deps of my plugins could become regular pluggy deps (and I could get some of my code being of some help for this ticket). How do I express explicit and implicit deps between plugins in pluggy today?

@RonnyPfannschmidt
Copy link
Member Author

@pombredanne

  1. a pluggy plugin can be a class instance, so the state can act on multiple hooks
  2. not at all

@pombredanne
Copy link
Contributor

@RonnyPfannschmidt ok, thanks!

So there is no special mechanism to express deps for a given hook (except for code and setup-level deps) ? just try_last and try_first?

Now on my side, I want to state explicitly that a plugin depends on another plugin to run.

Here is the sketch for me:

  • This will be expressed as a list of plugin names or possibly project:name as I am using multiple hook specs and stored as a class attribute of each plugin class.

  • In a given top-level run, each plugin can report whether it is "enabled" or not from receiving all the CLI kwargs (which are Click options FWIW). So for a main CLI function call, I will first collect the list of all plugins that are in the "enabled" state for that run. This is based on the plugin instance receiving the kwargs for the run and returning True or False.

  • I will then collect all the deps for each plugin, build a directed graph from these and:

    • check that there is no cycles in the deps graph. If not error out.
    • check that all the deps of a given plugin are there and enabled. If not error out.
  • Then I will do a topological sort on the graph: this gives me the execution precedence and order in which the plugins will then be called for each of the hooks specs (which each represent a "stage" in my scan processing pipeline).

  • Finally for each hook, I will call these hooks where needed in my processing and in the graph topo sort sequence

@RonnyPfannschmidt
Copy link
Member Author

@pombredanne currect, thats the state of things as they are,

note that tryfirst/trylast may sort different hooks of plugins into different locations, and hookwrappers also sort different than normal plugins

in addition "historic hooks" also behave differently

@pombredanne
Copy link
Contributor

@RonnyPfannschmidt so if you were to offer a way in pluggy to define dependencies between plugins (and I guess primarily in pytest) how and where would these be stated?

@pombredanne
Copy link
Contributor

pombredanne commented Jan 29, 2018

For the sake of clarity even if this is a repeat I am NOT talking about implicit code-level deps (e.g. from an import) and packaging-level deps (e.g. from a setup install_requires) which would both need to be there in the first place and are handled by Python and the setup/install machinery.

I am talking about runtime deps expressed between plugins to determine if and when a plugin should run given a running context when their code is already importable.

@RonnyPfannschmidt
Copy link
Member Author

@pombredanne honestly - no idea yet - i didn't work into the topic of expressing this topology, in particular since the effects on the details i mentioned are so tricky

@pombredanne
Copy link
Contributor

@RonnyPfannschmidt fair enough. I will keep trucking on my implementation then in anycase. And if and when you come down to adding this kind of deps to pluggy/pytest, it may give you some practical examples at the minimum.

One of the specific of my implementation --beside using Click for CLI-- is that I use multiple setuptools entry_points each corresponding to a PluginManager.

And I do not make use of naming conventions for plugin callables (e.g. prefixing function names with pytest_ or similar: instead a plugin is a class and must implement some methods with defined names and signatures that are the "hooks" that I will call. And for now I am not yet making use of the hooks features of pluggy, though I likely should/will in the future. But I am not sure I grok exactly how the hook registration and calling happens ;)

So my high level processing is:

  • load and collect plugins for each entrypoint. My plugins MUST use entry_points and it is not possible to load plugin in any other way (e.g. programmatically).
  • collect their declared Click options (a class attribute) and attach them to my Click command.
  • then run the Click command.
    • create an instance of each collected plugin class.
    • ask each instance if they are is_enabled passing them all the Click kwargs
    • (then I will resolve and check stated dependencies here)
    • if is_enabled, I call a setup method on the plugin instance (e.g. kinda like the _configure in pytest. (There is no need for any "unconfigure"/"teardown" for me for now)
    • when plugins need to be called, I call the supported method(s) for the project/entrypoint on each plugin of this project/entrypoint, passing all Click kwargs. And the order in which things will be called will be determined by the stated deps.

As examples, this is a simple plugin that returns the URLs found in a file. its "hook" is the get_scanner method. Or this plugin writes the final results to JSON and its "hook" is the process_codebase method. Each are for a different entry_point.

@goodboy goodboy mentioned this issue Apr 30, 2018
@goodboy
Copy link
Contributor

goodboy commented May 18, 2018

Just wanted to link to pytest-dev/pytest#3084 as a related deprecation due to this limitation.

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

No branches or pull requests

4 participants