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

[YANKED] pytest 8.1.0 removes many deprecations, but not mentioned in changelog #12069

Closed
mr-c opened this issue Mar 4, 2024 · 37 comments · Fixed by #12087
Closed

[YANKED] pytest 8.1.0 removes many deprecations, but not mentioned in changelog #12069

mr-c opened this issue Mar 4, 2024 · 37 comments · Fixed by #12087

Comments

@mr-c
Copy link

mr-c commented Mar 4, 2024

#11757 is not mentioned as a breaking change for the Pytest 8.1.0 Release in https://docs.pytest.org/en/stable/changelog.html ; it breaks plugins who were still supporting the older signature.

I'm opening this issue as a hint to others; is it possible to update the changelog after a release?

@RonnyPfannschmidt
Copy link
Member

this is a duplicate of #12068

the removal process itself is as intended, the oversight was that warnings of certain hooks where not displayed as expected

leaving this open for now to help people
the fact that it wasn't failing on 8.0 is the actual bug

@bluetech
Copy link
Member

bluetech commented Mar 4, 2024

@RonnyPfannschmidt @nicoddemus since we botched the deprecation, perhaps we should revert the removal until we can have at least one release with proper deprecation?

@RonnyPfannschmidt
Copy link
Member

should we yank 8.1 for the deprecation issues?

@RonnyPfannschmidt
Copy link
Member

lets defer the return of code a bit as i'd like to know first how to trigger the errors

@nicoddemus
Copy link
Member

I think it is fair to postpone the removals if we did not issue the proper warnings, as we vouch to do before any removals took place.

Those removals would then have to be postponed to 9.0/9.1 then.

Not sure if you folks would like to yank 8.1? Seems reasonable.

@bluetech
Copy link
Member

bluetech commented Mar 4, 2024

Reverting + releasing 8.1.1 should not be a problem, but if @RonnyPfannschmidt wants to avoid a revert, let's yank as it seems to be causing a lot of failures.

@RonnyPfannschmidt
Copy link
Member

lets yank and revert then

@nicoddemus
Copy link
Member

I'm yanking it.

@nicoddemus
Copy link
Member

image

@nicoddemus nicoddemus changed the title pytest 8.1.0 removes many deprecations, but not mentioned in changelog [YANKED] pytest 8.1.0 removes many deprecations, but not mentioned in changelog Mar 4, 2024
@nicoddemus nicoddemus pinned this issue Mar 4, 2024
@bluetech
Copy link
Member

bluetech commented Mar 4, 2024

Thanks @nicoddemus!

@RonnyPfannschmidt do you want me to revert the removal, or do you want to?

@nicoddemus
Copy link
Member

Opening PR to update the CHANGELOG.

@mr-c
Copy link
Author

mr-c commented Mar 4, 2024

Dear maintainers and devs, thank you for all that you do and the quick response! 💖

@sigma67
Copy link

sigma67 commented Mar 7, 2024

It is common practice in dependency update managers to bump the lower bound, so yanking is not a good idea imo.

It's probably better to leave the release up - if it breaks things, people will notice in their pipelines anyway.

@RonnyPfannschmidt
Copy link
Member

It seems the common practice is broken

Opportunistic bump of lower bounds is fatal

Lower bound should bump on feature needs

@nicoddemus
Copy link
Member

nicoddemus commented Mar 7, 2024

I'm a bit surprise by this practice of updating lower bounds automatically too.

Regardless, we will not revert the yank at this point -- it would only cause even more confusion.

We hope to get a new 8.1.1 soon, I will try to reserve some time later today to revert the removal of the deprecated features.

@nicoddemus
Copy link
Member

But in retrospect we probably should not have yanked the release.

Yanking should be used only in situations where the released package is broken/unusable, when (almost) every installation of the package will break the environment and/or runtime. In this case we only broke some plugins, but users not using those plugins were unaffected.

Live and learn I guess.

@alexted
Copy link

alexted commented Mar 9, 2024

But in retrospect we probably should not have yanked the release.

Yanking should be used only in situations where the released package is broken/unusable, when (almost) every installation of the package will break the environment and/or runtime. In this case we only broke some plugins, but users not using those plugins were unaffected.

Live and learn I guess.

Apparently, you were overnervous that you withdrew this version )))

@RonnyPfannschmidt
Copy link
Member

I still hold the opinion that yanking a release that enacts breakages whose warning where missing is a correct course of action

However I wasn't aware of wrongly overzealous compatibility pins, so in future I'll ensure a quick fix in the affected line is available for affected Parties

@sigma67
Copy link

sigma67 commented Mar 9, 2024

Edit: it seems I misunderstood how yanking works.

https://peps.python.org/pep-0592

@bluetech
Copy link
Member

bluetech commented Mar 9, 2024

@sigma67 No, if you pin the exact version, it should successfully install even if yanked. This is the famous "left-pad" issue.

@nicoddemus
Copy link
Member

nicoddemus commented Mar 9, 2024

@bluetech is correct AIFAK: yanking is not removing, the package is still available if you explicitly ask for it, for example:

λ pip install pytest==8.1.0
Collecting pytest==8.1.0
  Using cached pytest-8.1.0-py3-none-any.whl.metadata (7.6 kB)
Collecting iniconfig (from pytest==8.1.0)
  Using cached iniconfig-2.0.0-py3-none-any.whl.metadata (2.6 kB)
Requirement already satisfied: packaging in .\.env310\lib\site-packages (from pytest==8.1.0) (23.2)
Collecting pluggy<2.0,>=1.4 (from pytest==8.1.0)
  Using cached pluggy-1.4.0-py3-none-any.whl.metadata (4.3 kB)
Collecting exceptiongroup>=1.0.0rc8 (from pytest==8.1.0)
  Using cached exceptiongroup-1.2.0-py3-none-any.whl.metadata (6.6 kB)
Collecting tomli>=1 (from pytest==8.1.0)
  Using cached tomli-2.0.1-py3-none-any.whl.metadata (8.9 kB)
Collecting colorama (from pytest==8.1.0)
  Using cached colorama-0.4.6-py2.py3-none-any.whl.metadata (17 kB)
WARNING: The candidate selected for download or install is a yanked version: 'pytest' candidate (version 8.1.0 at https://files.pythonhosted.org/packages/5a/4a/3f626e3974bea1e6d471bd86f7965c67cd06d5770d1fec9aae445c44da7b/pytest-8.1.0-py3-none-any.whl (from https://pypi.org/simple/pytest/) (requires-python:>=3.8))
Reason for being yanked: See https://github.com/pytest-dev/pytest/issues/12069
Using cached pytest-8.1.0-py3-none-any.whl (335 kB)
Using cached exceptiongroup-1.2.0-py3-none-any.whl (16 kB)
Using cached pluggy-1.4.0-py3-none-any.whl (20 kB)
Using cached tomli-2.0.1-py3-none-any.whl (12 kB)
Using cached colorama-0.4.6-py2.py3-none-any.whl (25 kB)
Using cached iniconfig-2.0.0-py3-none-any.whl (5.9 kB)
Installing collected packages: tomli, pluggy, iniconfig, exceptiongroup, colorama, pytest
Successfully installed colorama-0.4.6 exceptiongroup-1.2.0 iniconfig-2.0.0 pluggy-1.4.0 pytest-8.1.0 tomli-2.0.1

Even though 8.1.0 was yanked -- locked dependencies should be fine then.

@edgarrmondragon
Copy link

Yup, I can confirm that poetry-locked yanked dependencies install just fine and merely emit a warning

@iurisilvio
Copy link

Is it expected to 8.1.1 proper warn about the deprecation?

The pytest-ruff don't trigger any deprecation warning. I don't know if it is expected or if I have to test it better.

@bluetech
Copy link
Member

@iurisilvio No, we still need to work on fixing the deprecation.

@bluetech
Copy link
Member

But you can test with the yanked 8.1.0 for now, if it works you're clear 😄

@iurisilvio
Copy link

Thanks! I'll test with 8.1.0!

@bwoodsend
Copy link

bwoodsend commented Mar 10, 2024

8.1.1 didn't revert all the removals, right? The _pytest.runner.call_runtest_hook removal is still waiving at me insistently using pytest 8.1.1.

Edit: I've remembered that that's private API so I guess it gets the well what did you expect? treatment.

@nicoddemus
Copy link
Member

nicoddemus commented Mar 10, 2024

No, we only reverted those two removals:

The rationale being that we were not warning about those deprecations.

call_runtest_hook was never deprecated because it was an internal API, which we do not guarantee stability across releases.

I guess it gets the well what did you expect? treatment.

Well perhaps not being so harsh 😁, but yes, we reserve the right to break those when we improve the internal code.

davvid referenced this issue in jsonpickle/jsonpickle Mar 12, 2024
* Fix #478 by whitelisting str for skipping read-only attrs

* Add changelog entry for str errors

* Move new fix from 3.0.3 to 3.0.4
flying-sheep pushed a commit to flying-sheep/pytest that referenced this issue Apr 9, 2024
flying-sheep pushed a commit to flying-sheep/pytest that referenced this issue Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants