Skip to content

Conversation

kdeldycke
Copy link
Collaborator

@kdeldycke kdeldycke commented Feb 23, 2024

This PR forces the context to close itself on CLI exits.

This makes sure all callbacks registered by invoked options are properly called whenever a ctx.exit() action is triggered. Which has the effect of preventing state leaks.

These leaks cannot be witness in standard CLI operations, because on ctx.exit() call, the OS process that is running the Python CLI will be destroyed.

Still, in unittests, these state leaks will leads to flaky and non-deterministic tests, as the pytest, tox or any other test framework will keep using the same Python interpreter. To demonstrate this, I implemented a unit test using loggers as a fixture, in addition to a generic unit test.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

kdeldycke added a commit to kdeldycke/click-extra that referenced this pull request Feb 23, 2024
@kdeldycke
Copy link
Collaborator Author

Note that this might be related to: #1053

@aenglander aenglander added this to the 8.2.0 milestone May 20, 2024
@AndreasBackx AndreasBackx mentioned this pull request Oct 20, 2024
34 tasks
kdeldycke and others added 2 commits November 3, 2024 12:24
[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Add changelog entry

Lint
@AndreasBackx AndreasBackx merged commit c326df9 into pallets:main Nov 3, 2024
12 checks passed
@kdeldycke kdeldycke deleted the context-close-before-exit branch November 3, 2024 14:31
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants