fix: Clear stale negative cache entries#4328
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4328 +/- ##
==========================================
+ Coverage 55.08% 55.95% +0.86%
==========================================
Files 240 240
Lines 30329 30375 +46
Branches 3241 3250 +9
==========================================
+ Hits 16707 16995 +288
+ Misses 13619 13377 -242
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| void update_monitor(progress_callback_t* cb, PackageExtractEvent event) const; | ||
|
|
||
| specs::PackageInfo m_package_info; | ||
| MultiPackageCache* m_caches = nullptr; |
There was a problem hiding this comment.
Hmm is this a leftover? It doesn't seem to be used and I'm not sure I understand why it would be needed.
There was a problem hiding this comment.
The MultiPackageCache is actually used via the PackageFetchers (see their instantiations bellow).
| struct ScopedContextOverride | ||
| { | ||
| ScopedContextOverride(Context& ctx, const fs::u8path& prefix, const fs::u8path& pkgs_dir) | ||
| { | ||
| m_ctx = &ctx; | ||
| m_saved_prefix_params = ctx.prefix_params; | ||
| m_saved_pkgs_dirs = ctx.pkgs_dirs; | ||
| m_saved_prefix_data_interoperability = ctx.prefix_data_interoperability; | ||
| m_saved_extract_threads = ctx.threads_params.extract_threads; | ||
|
|
||
| ctx.prefix_params.target_prefix = prefix; | ||
| ctx.prefix_params.root_prefix = prefix; | ||
| ctx.prefix_params.conda_prefix = prefix; | ||
| ctx.prefix_params.relocate_prefix = prefix; | ||
| ctx.pkgs_dirs = { pkgs_dir }; | ||
| ctx.prefix_data_interoperability = false; | ||
| ctx.threads_params.extract_threads = 1; | ||
| } | ||
|
|
||
| ~ScopedContextOverride() | ||
| { | ||
| m_ctx->prefix_params = m_saved_prefix_params; | ||
| m_ctx->pkgs_dirs = m_saved_pkgs_dirs; | ||
| m_ctx->prefix_data_interoperability = m_saved_prefix_data_interoperability; | ||
| m_ctx->threads_params.extract_threads = m_saved_extract_threads; | ||
| } | ||
|
|
||
| Context* m_ctx = nullptr; | ||
| PrefixParams m_saved_prefix_params; | ||
| std::vector<fs::u8path> m_saved_pkgs_dirs; | ||
| bool m_saved_prefix_data_interoperability = false; | ||
| int m_saved_extract_threads = 0; | ||
| }; |
There was a problem hiding this comment.
This seems overrated and doesn't seem to be used either (unused declared context_guard).
There was a problem hiding this comment.
The constructor and the destructor of ScopedContextOverride are handling the logic of the interaction with the context guaranteeing that the state will be the original one when the scope change.
So context_guard is declared (also defined) unused by any other instructions, yet not useless.
We have this pattern at several places in tests.
There was a problem hiding this comment.
We have this pattern at several places in tests.
In that case, we may want to consider having a global common struct for all tests instead of rewriting it in each test (even if not all parameters are involved each time).
To be evaluated...
jjerphan
force-pushed
the
fix/clear-negative-cache-entries
branch
from
67b4f4c to
1485058
Compare
jjerphan
force-pushed
the
fix/clear-negative-cache-entries
branch
from
fc32cc4 to
1659911
Compare
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Johan Mabille <johan.mabille@gmail.com>
3 participants
Description
Tentative fix for #4322.
Fix failures observed in Code Coverage workflows.
Type of Change
Checklist
pre-commit run --alllocally in the source folder and confirmed that there are no linter errors.