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.

Sign up for GitHub

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

Jump to bottom

Re-factor cache invalidation #374

Closed
sloria opened this issue Mar 11, 2019 · 2 comments · Fixed by #462
Closed

Re-factor cache invalidation #374

sloria opened this issue Mar 11, 2019 · 2 comments · Fixed by #462
Labels
backwards incompatible enhancement
Milestone
6.0.0

Comments

@sloria
Copy link
Member

sloria commented Mar 11, 2019

#371 (comment)
@sloria sloria mentioned this issue Mar 11, 2019
Closed
@lafrech lafrech mentioned this issue Mar 15, 2019
Closed
@lafrech lafrech added this to the 6.0.0 milestone Aug 20, 2019
pull bot pushed a commit to DuncanBetts/webargs that referenced this issue Oct 30, 2019
@dependabot @sloria
Bump tox from 3.3.0 to 3.4.0 (marshmallow-code#293)
8a1c349 
Bumps [tox](https://github.com/tox-dev/tox) from 3.3.0 to 3.4.0.
<details>
<summary>Changelog</summary>

*Sourced from [tox's changelog](https://github.com/tox-dev/tox/blob/master/CHANGELOG.rst).*

> 3.4.0 (2018-09-20)
> ------------------
> 
> Bugfixes
> ^^^^^^^^
> 
> - add ``--exists-action w`` to default pip flags to handle better VCS dependencies (`pip documentation on this <https://pip.pypa.io/en/latest/reference/pip/#exists-action-option>`_) - by :user:`gaborbernat` (`marshmallow-code#503 <https://github-redirect.dependabot.com/tox-dev/tox/issues/503>`_)
> - instead of assuming the Python version from the base python name ask the interpreter to reveal the version for the ``ignore_basepython_conflict`` flag - by :user:`gaborbernat` (`marshmallow-code#908 <https://github-redirect.dependabot.com/tox-dev/tox/issues/908>`_)
> - PEP-517 packaging fails with sdist already exists, fixed via ensuring the dist folder is empty before invoking the backend and `pypa/setuptools 1481 <https://github-redirect.dependabot.com/pypa/setuptools/pull/1481>`_ - by :user:`gaborbernat` (`#1003 <https://github-redirect.dependabot.com/tox-dev/tox/issues/1003>`_)
> 
> 
> Features
> ^^^^^^^^
> 
> - add ``commands_pre`` and ``commands_post`` that run before and after running
>   the ``commands`` (setup runs always, commands only if setup suceeds, teardown always - all
>   run until the first failing command)  - by :user:`gaborbernat` (`marshmallow-code#167 <https://github-redirect.dependabot.com/tox-dev/tox/issues/167>`_)
> - ``pyproject.toml`` config support initially by just inline the tox.ini under ``tool.tox.legacy_tox_ini`` key; config source priority order is ``pyproject.toml``, ``tox.ini`` and then ``setup.cfg`` - by :user:`gaborbernat` (`marshmallow-code#814 <https://github-redirect.dependabot.com/tox-dev/tox/issues/814>`_)
> - use the os environment variable ``TOX_SKIP_ENV`` to filter out tox environment names from the run list (set by ``envlist``)  - by :user:`gaborbernat` (`marshmallow-code#824 <https://github-redirect.dependabot.com/tox-dev/tox/issues/824>`_)
> - always set ``PIP_USER=0`` (do not install into the user site package, but inside the virtual environment created) and ``PIP_NO_DEPS=0`` (installing without dependencies can cause broken package installations) inside tox - by :user:`gaborbernat` (`marshmallow-code#838 <https://github-redirect.dependabot.com/tox-dev/tox/issues/838>`_)
> - tox will inject some environment variables that to indicate a command is running within tox: ``TOX_WORK_DIR`` env var is set to the tox work directory,
>   ``TOX_ENV_NAME`` is set to the current running tox environment name, ``TOX_ENV_DIR`` is set to the current tox environments working dir - by :user:`gaborbernat` (`marshmallow-code#847 <https://github-redirect.dependabot.com/tox-dev/tox/issues/847>`_)
> - While running tox invokes various commands (such as building the package, pip installing dependencies and so on), these were printed in case they failed as Python arrays. Changed the representation to a shell command, allowing the users to quickly replicate/debug the failure on their own - by :user:`gaborbernat` (`marshmallow-code#851 <https://github-redirect.dependabot.com/tox-dev/tox/issues/851>`_)
> - skip missing interpreters value from the config file can now be overridden via the ``--skip-missing-interpreters`` cli flag - by :user:`gaborbernat` (`marshmallow-code#903 <https://github-redirect.dependabot.com/tox-dev/tox/issues/903>`_)
> - keep additional environments config order when listing them - by :user:`gaborbernat` (`marshmallow-code#921 <https://github-redirect.dependabot.com/tox-dev/tox/issues/921>`_)
> - allow injecting config value inside the ini file dependent of the fact that we're connected to an interactive shell or not  - by :user:`gaborbernat` (`#947 <https://github-redirect.dependabot.com/tox-dev/tox/issues/947>`_)
> - do not build sdist if skip install is specified for the envs to be run - by :user:`gaborbernat` (`#974 <https://github-redirect.dependabot.com/tox-dev/tox/issues/974>`_)
> - when verbosity level increases above two start passing through verbosity flags to pip - by :user:`gaborbernat` (`#982 <https://github-redirect.dependabot.com/tox-dev/tox/issues/982>`_)
> - when discovering the interpreter to use check if the tox host Python matches and use that if so - by :user:`gaborbernat` (`#994 <https://github-redirect.dependabot.com/tox-dev/tox/issues/994>`_)
> - ``-vv`` will print out why a virtual environment is re-created whenever this operation is triggered - by :user:`gaborbernat` (`#1004 <https://github-redirect.dependabot.com/tox-dev/tox/issues/1004>`_)
> 
> 
> Documentation
> ^^^^^^^^^^^^^
> 
> - clarify that ``python`` and ``pip`` refer to the virtual environments executable - by :user:`gaborbernat` (`marshmallow-code#305 <https://github-redirect.dependabot.com/tox-dev/tox/issues/305>`_)
> - add Sphinx and mkdocs example of generating documentation via tox - by :user:`gaborbernat` (`marshmallow-code#374 <https://github-redirect.dependabot.com/tox-dev/tox/issues/374>`_)
> - specify that ``setup.cfg`` tox configuration needs to be inside the ``tox:tox`` namespace - by :user:`gaborbernat` (`marshmallow-code#545 <https://github-redirect.dependabot.com/tox-dev/tox/issues/545>`_)
</details>
<details>
<summary>Commits</summary>

- [`b6f23aa`](tox-dev/tox@b6f23aa) release 3.4.0
- [`9a006f7`](tox-dev/tox@9a006f7) Fixup changelog and write live config content when updating it ([#1006](https://github-redirect.dependabot.com/tox-dev/tox/issues/1006))
- [`4269996`](tox-dev/tox@4269996) Add pre and post commands ([#1000](https://github-redirect.dependabot.com/tox-dev/tox/issues/1000))
- [`d29829e`](tox-dev/tox@d29829e) Added Python 3.7 to trove classifiers
- [`cf6afce`](tox-dev/tox@cf6afce) config.rst: Improve the wording of the introduction
- [`70bfb83`](tox-dev/tox@70bfb83) https-ify some links
- [`db2f661`](tox-dev/tox@db2f661) fallback to current interpreter if matches at discovery
- [`768f412`](tox-dev/tox@768f412) tox injects env vars to indicate running within tox [marshmallow-code#847](https://github-redirect.dependabot.com/tox-dev/tox/issues/847)
- [`838dfb0`](tox-dev/tox@838dfb0) tox.ini: testenv: remove --junitxml
- [`1d977fa`](tox-dev/tox@1d977fa) drop old style exception get, use str.format instead of +
- Additional commits viewable in [compare view](tox-dev/tox@3.3.0...3.4.0)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=tox&package-manager=pip&previous-version=3.3.0&new-version=3.4.0)](https://dependabot.com/compatibility-score.html?dependency-name=tox&package-manager=pip&previous-version=3.3.0&new-version=3.4.0)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

Dependabot will **not** automatically merge this PR because it includes a minor update to a production dependency.

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
@sirosen
Copy link
Collaborator

sirosen commented Jan 24, 2020

Is it reasonable to consider removing the cache altogether?
I think that after #420 , it maybe isn't so important to cache the results of parsing.

Alternatively, I would be willing to take a crack at the suggested solution,

Changing Parser classes into parser class factories, making use_args, use_kwargs, and perhaps parse class methods.

to see if what I come up with is satisfactory.

@lafrech
Copy link
Member

lafrech commented Jan 24, 2020

Is it reasonable to consider removing the cache altogether?
I think that after #420 , it maybe isn't so important to cache the results of parsing.

Good point. Is there any interest in keeping a cache, now?

sirosen added a commit to sirosen/webargs that referenced this issue Jan 24, 2020
@sirosen
Remove the Parser cache
f2071b0 
Because the cache is no longer used field-by-field to fetch data,
there's significantly less value in keeping it. Combined with the fact
that each parser instantiation was already clearing the cache to avoid
a security bug ( marshmallow-code#371 ), the cache is no longer actually used at all
in most (any?) contexts.

Remove the cache and all of the machinery associated with it
(Parser._clear_cache, Parser._clone, and relevant checks).

Resolves marshmallow-code#374
@sirosen sirosen mentioned this issue Jan 24, 2020
Merged
sirosen added a commit to sirosen/webargs that referenced this issue Jan 30, 2020
@sirosen
Remove the Parser cache
96025bc 
Because the cache is no longer used field-by-field to fetch data,
there's significantly less value in keeping it. Combined with the fact
that each parser instantiation was already clearing the cache to avoid
a security bug ( marshmallow-code#371 ), the cache is no longer actually used at all
in most (any?) contexts.

Remove the cache and all of the machinery associated with it
(Parser._clear_cache, Parser._clone, and relevant checks).

Resolves marshmallow-code#374
sirosen added a commit to sirosen/webargs that referenced this issue Jan 30, 2020
@sirosen
Remove the Parser cache
6794dc2 
Because the cache is no longer used field-by-field to fetch data,
there's significantly less value in keeping it. Combined with the fact
that each parser instantiation was already clearing the cache to avoid
a security bug ( marshmallow-code#371 ), the cache is no longer actually used at all
in most (any?) contexts.

Remove the cache and all of the machinery associated with it
(Parser._clear_cache, Parser._clone, and relevant checks).

Resolves marshmallow-code#374
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backwards incompatible enhancement
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

Remove the Parser cache sirosen/webargs
3 participants
@sirosen @lafrech @sloria