Add name parameter to PluginManager.load_setuptools_entrypoints

[nicoddemus]

Related to pytest-dev/pytest#4727

[RonnyPfannschmidt]

im wondering if we ought to consider the loading/naming of plugins for a own object that manages it
this would allow to expand further into having auto-loading plugins vs requestable but named plugin

it could also enable reordering by request before autoloading

[nicoddemus]

@RonnyPfannschmidt can you elaborate a little more what you mean? I can kind of guess what you mean, but would like to avoid having to guess and get the gist directly instead. 😉

Also I assume the topic you're bringing up is for another PR, not something to be done in this one. 👍

[RonnyPfannschmidt]

yes, this is actually better put up as a followup,

the key idea is to encapsulate plugin naming and loading in a way that enables not just affecting the order of entry-point loading, but also enables to create entry-points for opt-in plugins

also it would enable to abstract the loader backend, after all, @asottile demonstrated in pre-commit that the entry-points package can shave quite some time off startup, and thats something we want to have for consumers of pluggy, in particular consumers like pytest and tox, where shaving off might safe users quite a while of time

same goes for deeper control of what plugins to load from where

[asottile]

for completeness, here's where I replaced pkg_resources with entrypoints for flake8 resulting in a 26.6% startup savings: https://gitlab.com/pycqa/flake8/merge_requests/264#note_118110874

[nicoddemus]

Ready for review!

Once this gets merged, I plan to release 0.9 with this in order to finish pytest-dev/pytest#4727. 👍

[goodboy]

for count, ep in enumerate(iter_entry_points(group, name=name)): maybe a little cleaner?

[RonnyPfannschmidt]

it would be wrong ^^ - see the continue cases

[goodboy]

a good call. missed that cause of the stacked diff 😖

[goodboy]

@nicoddemus one little comment but looks good.

[asottile]

this might make #190 more difficult to implement 🤔

[nicoddemus]

this might make #190 more difficult to implement

Shouldn't really, should be a matter of just adapting the iteration to return only the entry points requested. See the original implementation:

https://github.com/pypa/pkg_resources/blob/6f81a44010d1266494025647dd1e1f0befa5b26b/pkg_resources/__init__.py#L743

[asottile]

this might make #190 more difficult to implement

Shouldn't really, should be a matter of just adapting the iteration to return only the entry points requested. See the original implementation:

https://github.com/pypa/pkg_resources/blob/6f81a44010d1266494025647dd1e1f0befa5b26b/pkg_resources/__init__.py#L743

importlib_metadata doesn't currently have an api for that as far as I know :S - I can copy the implementation of their current api and stick an if in the inner loop but it might not be pretty :(

[nicoddemus]

It is just a matter of skipping the entry points with that name. The current code is:

pluggy/pluggy/manager.py

Lines 269 to 272 in 3c384dd

	for ep in iter_entry_points(group, name=name):
	# is the plugin registered or blocked?
	if self.get_plugin(ep.name) or self.is_blocked(ep.name):
	continue

Unless I'm missing something, based on #190, we would need to change it to:

        for ep in importlib_metadata.entry_points().get(entrypoint_name, ()):
            # only get entry points with the given name
            if name is not None and ep.name != name:
                continue
            # is the plugin registered or blocked?
            if self.get_plugin(ep.name) or self.is_blocked(ep.name):
                continue

[asottile]

ah yeah the difference there is that does a full scan of site-packages per call

[nicoddemus]

You mean iter_entry_points doesn't? I didn't look too deep, but the implementation doesn't seem to do anything to avoid doing a full scan:

    def iter_entry_points(self, group, name=None):
        """Yield entry point objects from `group` matching `name`
        If `name` is None, yields all entry points in `group` from all
        distributions in the working set, otherwise only ones matching
        both `group` and `name` are yielded (in distribution order).
        """
        for dist in self:
            entries = dist.get_entry_map(group)
            if name is None:
                for ep in entries.values():
                    yield ep
            elif name in entries:
                yield entries[name]

Anyway even if it does a full-scan, it is not a big-deal for pytest's case because the name parameter is only given for each -p call given in the command-line, which I suspect will be only 1 or 2, tops.

[asottile]

pkg_resources builds the set as an import side-effect (most of the rationale for why it is slow and why we want to eliminate it) -- the apis are then just an iteration over the in-memory mappings

[nicoddemus]

Oh OK, thanks for the explanation!

It is not clear to me yet why this change would make it harder for us to land importlib_metadata later though... 🤔

[asottile]

we'll see, I'll do some profiling once importlib-metadata 0.9 gets released 🙃

[nicoddemus]

Oh you mean there might be a performance issue? Sorry, I thought it wouldn't be possible, that's why I was curious.

Hmm I'm just seeing that the name parameter has been removed in https://gitlab.com/python-devs/importlib_metadata/merge_requests/39... that's the reason of your unease about this change? Perhaps we can contribute implementing it?

[asottile]

I suspect the answer might be building a global cache inside pluggy, but I'll wait until the release happens to poke around -- nothing urgent I think

[nicoddemus]

I suspect the answer might be building a global cache inside pluggy, but I'll wait until the release happens to poke around -- nothing urgent I think

Sure thing