Upgrading pluggy to 1.1.0 causes KeyError
sweettyler opened this issue · 19 comments
Upgrading pluggy from "1.0.0 pyhd8ed1ab_5 conda-forge" to"1.1.0 pyhd8ed1ab_0 conda-forge" causes
KeyError('pkgs_dirs')
An unexpected error has occurred. Conda has prepared the above report.
Thanks @sweettyler!
Similar report on the conda-forge package: conda-forge/pluggy-feedstock#34
Didn't know conda uses pluggy. I'll try to understand why it fails.
In the meantime, would you be able to mark the conda package as broken?
The problem happens because of this hook impl in conda:
It uses yield but is not a hookwrapper, which now becomes a new-style wrapper (added in pluggy 1.1.0), which requires a return value, which is None, hence the TypeError: 'NoneType' object is not iterable error.
The conda hookspec is def conda_subcommands(self) -> Iterable[CondaSubcommand] and the hookimpl uses yield as an easy way to return an Iterable, I did not anticipate this usage.
I guess pluggy 1.1.0 broke a valid use, so we should revert the "new-style wrappers" idea for now and reconsider. Either through a 1.1.1 release, or yanking 1.1.0 (note I don't have permissions to yank).
@nicoddemus WDYT?
Yanking is fine.
I suppose you mean yanking both pypi and conda releases, correct?
Yes
It uses yield but is not a hookwrapper, which now becomes a new-style wrapper (added in pluggy 1.1.0), which requires a return value, which is None, hence the TypeError: 'NoneType' object is not iterable error.
A solution to this might be to require to be marked as a hookwrapper (like before), instead of being a hookwrapper implicitly just by being a generator.
Yes
I will do it then, thanks for the quick look!
Yanked https://pypi.org/project/pluggy/1.1.0.
Will start the process now for the conda package.
Thanks for the quick response, this breaks a lot, especially since the conda-forge recipe was updated so quick :)
Thanks @nicoddemus.
If the yanking ends up being good enough, I'll work on a pluggy 1.2.0 which reworks the feature (probably using an explicit wrapper=True), otherwise I can do a quick 1.1.1 release which just reverts the feature.
For now I'll assume the yanks are good, but let me know if not.
Sure, I'm sure yanking will be fine.
I will open a PR here updating the CHANGELOG.
😍
So we hit hyrums law, glad a yank and a named marker will be enough to resolve that one
Ironic, given that conda is trying to escape Hyrum using plugins 🤣😭
BTW, in case you need any more details about why we're doing this:
- conda enhancement proposals that propose plugins: CEP 2, CEP 4
- CEP 3 for the first plugin to port the mamba solver to conda
- developer docs about conda plugins (or start of them at least)
- Anaconda announcement (even though the feature was a community-driven effort)
- release blog post
- PyCon US talk from conda co-maintainer @beeankha
- PyCon DE talk from conda co-maintainer @travishathaway (no video yet?)
Thanks @jezdez for the context.
Perhaps we can close this, given the issue itself has been solved, and open a new one to track the change in the new-style hook wrappers as per #403 (comment)?
Thanks @jezdez for the context.
No problem, I apologize that I hadn't the foresight to actively reach out to you before this happened, and wanted to thank you for your swift response here.
Perhaps we can close this, given the issue itself has been solved, and open a new one to track the change in the new-style hook wrappers as per #403 (comment)?
Perfect!
@jezdez Conda using pluggy is great. I think we should add conda (and other projects using pluggy besides pytest) to our pre-release testing. I'll open an issue about that.
@nicoddemus I'll close the issue, we can use a different issue for the rework.