-
Notifications
You must be signed in to change notification settings - Fork 0
Cache dependency version fetches per invocation #552
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import logging | ||
| import os | ||
| from functools import cache | ||
| from pathlib import Path | ||
| from shutil import which | ||
| from typing import Union | ||
|
|
@@ -63,17 +64,19 @@ def auto_path() -> Path: | |
| return context | ||
|
|
||
|
|
||
| def cached_session(**kwargs) -> CachedSession: | ||
| """Create a cached requests session with default settings.""" | ||
| session_kwargs = { | ||
| "cache_name": "bakery_cache", | ||
| "expire_after": 3600, | ||
| "backend": "filesystem", | ||
| "use_temp": True, | ||
| "allowable_methods": ["GET"], | ||
| "allowable_codes": [200], | ||
| "stale_if_error": True, | ||
| } | ||
| session_kwargs.update(kwargs) | ||
| @cache | ||
| def cached_session() -> CachedSession: | ||
|
Comment on lines
+67
to
+68
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't need to test core language functionality IMO. This isn't a complicated setup. |
||
| """Return a process-wide cached requests session. | ||
|
|
||
| return CachedSession(**session_kwargs) | ||
| Memoized so backend initialization and the accompanying log chatter only | ||
| happen once per bakery invocation. | ||
| """ | ||
| return CachedSession( | ||
| cache_name="bakery_cache", | ||
| expire_after=3600, | ||
| backend="filesystem", | ||
| use_temp=True, | ||
| allowable_methods=["GET"], | ||
| allowable_codes=[200], | ||
| stale_if_error=True, | ||
| ) | ||
|
Comment on lines
+74
to
+82
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a crazy idea: what if we cached this somehow as part of |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,6 +92,21 @@ def patch_testdata_response(url: str): | |
|
|
||
| @pytest.fixture(scope="function") | ||
| def disable_requests_caching(mocker): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be autouse? |
||
| # The session factory and fetch helpers are decorated with @functools.cache, | ||
| # so values returned by an earlier test would otherwise survive into the | ||
| # next one. Clear the per-process caches so each test's patches take effect. | ||
| from posit_bakery.config.dependencies.positron import PositronDependency | ||
| from posit_bakery.config.dependencies.python import PythonDependency | ||
| from posit_bakery.config.dependencies.quarto import QuartoDependency | ||
| from posit_bakery.config.dependencies.r import RDependency | ||
| from posit_bakery.util import cached_session | ||
|
|
||
| cached_session.cache_clear() | ||
| PythonDependency._fetch_versions.cache_clear() | ||
| RDependency._fetch_versions.cache_clear() | ||
| QuartoDependency._fetch_versions.cache_clear() | ||
| PositronDependency._fetch_versions.cache_clear() | ||
|
|
||
| return mocker.patch("posit_bakery.util.CachedSession", spec=requests.Session) | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
roborev noted that the cached result can be mutated. We may want to note that or find a way to always return a copy.