Skip to content

Conversation

@blueyed
Copy link
Contributor

@blueyed blueyed commented Apr 3, 2019

Based on #5033

@nicoddemus
Copy link
Member

I'm unsure if this is the correct approach for the use case mentioned in #5033; seems strange to disable the entire cache (which a lot of plugins might be using) if you want to disable --lf.

On the other hand, this seems like a useful option for other situations, for example when people try to write to read-only file systems: #1029. This is more elegant than disabling the entire cacheprovider plugin, because in that case you don't have the config.cache object anymore which might break other plugins which use the cache.

So 👍 to this.

@nicoddemus
Copy link
Member

Besides tests and changelog, we should also add this to the documentation, both in cache.rst as an example and to reference.rst.

@RonnyPfannschmidt
Copy link
Member

i believe this breaks the expectations of the system in strange ways,
i consier it a breaking api change as it changes cache writes, that may be followed by cache reads a noop that generate a "dirty/outdated read" situation

@blueyed
Copy link
Contributor Author

blueyed commented Sep 13, 2019

ok, closing for now.

@blueyed blueyed closed this Sep 13, 2019
@blueyed blueyed deleted the cache-readonly branch September 13, 2019 13:18
@blueyed blueyed mentioned this pull request Oct 21, 2019
@pytest-dev pytest-dev deleted a comment from codecov bot Nov 2, 2019
blueyed added a commit to blueyed/pytest that referenced this pull request Nov 2, 2019
I was about to add `--lf-noupdate`, when I've remembered this idea.

Rejected in pytest-dev#5043
Based on pytest-dev#5033
@blueyed
Copy link
Contributor Author

blueyed commented Nov 9, 2019

For reference: this allows for setting it read-only internally with failures (0e33845).
This turned out to be useful in tests when there are other plugins installed (before I've disabled them for inner runs), since the number of warnings then differs.

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 this pull request may close these issues.

3 participants