feat: support for reuse_venv option by samypr100 · Pull Request #730 · wntrblm/nox

samypr100 · 2023-07-24T02:38:21Z

Closes #488

This PR introduces a new option reuse_venv at the CLI and nox.options level as discussed in #488. It takes 4 possible values, namely no, yes, never, and always as discussed in the issue.
The code that evaluates if a venv should be reused has been switched to use this new option, and backwards compatiblity with reuse_existing_virtualenvs is kept using appropriate finalizer_func and merge_func on _options.py.

I've added tests to test new functionality as well do backwards compatibility checks and hopefully 🤞 updated the docs properly. I also added a new fixture generate_noxfile_options that helps with more dynamic configuration of resources/noxfile_options.py used in the tests to reduce boilerplate.

johnthagen · 2023-08-21T12:04:46Z

@theacodes We're excited to get to use this feature, was wondering if you were able to review or your thoughts? Thanks!

johnthagen · 2023-11-07T14:37:11Z

@FollowTheProcess Would you be willing to review this MR? Thanks.

funkyfuture · 2023-12-16T16:18:51Z

i only looked on the user docs and i'm wondering:

could always and never be used for nox.options.reuse_venv?
if so, do these values make any sense at that level / should they be disallowed to protect users from confusion?

thanks for the feature implementation!

funkyfuture · 2023-12-17T12:57:57Z

the prose about the feature and its usage is fine, but as a user i think the section would aso benefit from a summary such as #488 (comment) or a table.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii · 2024-02-24T03:01:39Z

--reuse-venv is a bit longer then --reuse (and you always have to specify a value, too, so it's a little lengthy on the command line, but the current long option is --reuse-existing-virtualenvs. The option draws a parallel with reuse_venv: bool = ... in the sessions, so I would be okay with the longer from, but wouldn't object at all to the shorter --reuse either, especially since there's not a short option for it.
Anyone have thoughts? Otherwise, this looks good to me.

cjolowicz · 2024-02-24T19:56:12Z

+                # sessions marked True will always be reused unless never is specified
+                self.func.reuse_venv is True
+                and self.global_config.reuse_venv != "never",
+                # session marked False will never be reused unless always is specified


This comment doesn't relate to the code below

I tried to clarify this function with even more documentation here 715bc9a, let me know if it's better.

This is super helpful

cjolowicz · 2024-02-24T20:24:20Z

        ),
        group=options.groups["environment"],
-        help="Reuse existing virtualenvs instead of recreating them.",
+        help="This is an alias for '--reuse-venv=yes|no'.",


I'd love to see this called out in the docs as well

It should be in the docs, near the end of this statement in config.rst:

* ``nox.options.reuse_existing_virtualenvs`` is equivalent to specifying :ref:--reuse-existing-virtualenvs . You can force this off by specifying ``--no-reuse-existing-virtualenvs`` during invocation. Alias of ``nox.options.reuse_venv``.

The docs should state explicitly that --reuse-venv=yes is equivalent to --reuse-existing-virtualenvs.

I'm happy for this to land as-is though. We probably want to deprecate the longer version anyway.

(OT: It might also be a nice improvement to allow specifying --reuse-venv without a value, defaulting to yes.)

Agree, I didn't explicitly deprecate it on this PR but it can be done so in a follow up one. When that happens I think it makes sense to augment --reuse-venv (or add --reuse/--no-reuse like @henryiii prefers) to default to yes when specified. Note -r is a valid shorthand for --reuse-venv=yes since it's a shorcut to --reuse-existing-virtualenvs .

The docs should state explicitly that --reuse-venv=yes is equivalent to --reuse-existing-virtualenvs.

Done in fdf876c and 88d11b7

cjolowicz · 2024-02-24T20:28:24Z

Personally I like the more explicit --reuse-venv. There are other things one might "reuse" (wheels?). AFAIU -r also becomes a short option for --reuse-venv=yes since it's equivalent.

…virtualenvs is alias to --reuse-venv=yes|no in usage.rst

johnthagen reviewed Jul 24, 2023

View reviewed changes

Comment thread nox/_options.py

edgarrmondragon reviewed Nov 19, 2023

View reviewed changes

Comment thread tests/test_tasks.py Outdated

edgarrmondragon reviewed Nov 21, 2023

View reviewed changes

Comment thread nox/_options.py Outdated

henryiii mentioned this pull request Feb 19, 2024

List of PRs that are ready for review #769

Closed

samypr100 added 6 commits February 23, 2024 12:51

feat: support for reuse_venv option

7997f3e

typing: adding Literal for reuse_venv choices

a13dd11

chore: ruff-format

3efc4c1

chore: codespell

397d89d

chore: move generate_noxfile_options fixture to conftest.py

4eb4c4e

chore: ruff-format

0085e2b

henryiii force-pushed the new-global-option-reuse-venv branch from 24f7c39 to 0085e2b Compare February 23, 2024 17:53

chore: fix lints

97e21ee

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

henryiii approved these changes Feb 24, 2024

View reviewed changes

cjolowicz reviewed Feb 24, 2024

View reviewed changes

samypr100 added 3 commits February 24, 2024 20:17

Merge branch 'wntrblm:main' into new-global-option-reuse-venv

12f01f3

docs: clarify reuse_existing_venv function decision matrix

715bc9a

chore: remove `# pragma: no cover due to recent main branch changes

61dd272

cjolowicz approved these changes Feb 25, 2024

View reviewed changes

samypr100 added 2 commits February 25, 2024 08:25

docs: call out that --reuse-existing-virtualenvs/--no-reuse-existing-…

fdf876c

…virtualenvs is alias to --reuse-venv=yes|no in usage.rst

docs: small update to config.rst

88d11b7

cjolowicz merged commit 5a097cd into wntrblm:main Feb 25, 2024

samypr100 deleted the new-global-option-reuse-venv branch November 3, 2024 23:49

Uh oh!

Conversation

samypr100 commented Jul 24, 2023

Uh oh!

Uh oh!

johnthagen commented Aug 21, 2023

Uh oh!

johnthagen commented Nov 7, 2023

Uh oh!

Uh oh!

Uh oh!

funkyfuture commented Dec 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

funkyfuture commented Dec 17, 2023

Uh oh!

henryiii commented Feb 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjolowicz Feb 24, 2024

Choose a reason for hiding this comment

Uh oh!

samypr100 Feb 25, 2024

Choose a reason for hiding this comment

Uh oh!

cjolowicz Feb 25, 2024

Choose a reason for hiding this comment

Uh oh!

cjolowicz Feb 24, 2024

Choose a reason for hiding this comment

Uh oh!

samypr100 Feb 25, 2024

Choose a reason for hiding this comment

Uh oh!

cjolowicz Feb 25, 2024

Choose a reason for hiding this comment

Uh oh!

samypr100 Feb 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samypr100 Feb 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjolowicz commented Feb 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

funkyfuture commented Dec 16, 2023 •

edited

Loading

henryiii commented Feb 24, 2024 •

edited

Loading

samypr100 Feb 25, 2024 •

edited

Loading

samypr100 Feb 25, 2024 •

edited

Loading

cjolowicz commented Feb 24, 2024 •

edited

Loading