Skip to content

Commit 6da1431

Browse files
fix(pypi): correctly write the used facts back (bazel-contrib#3719)
Summary: 1. Added 3 new tests to pypi_cache_tests.bzl 2. Fixed `_cache` helper to handle both struct and dict return values 3. Fixed `_filter_packages`: Returns None for empty dict results instead of empty dict 4. Fixed `_get_from_facts`: Now stores facts when reading index_urls from the dict case Fixes bazel-contrib#3711 Fixes bazel-contrib#3707 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent fe43548 commit 6da1431

2 files changed

Lines changed: 104 additions & 2 deletions

File tree

python/private/pypi/pypi_cache.bzl

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,12 @@ def _filter_packages(dists, requested_versions):
128128
return dists
129129

130130
if type(dists) == "dict":
131-
return {
131+
result = {
132132
pkg: url
133133
for pkg, url in dists.items()
134134
if pkg in requested_versions
135135
}
136+
return result if result else None
136137

137138
sha256s_by_version = {}
138139
whls = {}
@@ -206,10 +207,13 @@ def _get_from_facts(facts, known_facts, index_url, requested_versions, facts_ver
206207
return None
207208

208209
if type(requested_versions) == "dict":
209-
return _filter_packages(
210+
result = _filter_packages(
210211
dists = known_facts.get("index_urls", {}).get(index_url, {}),
211212
requested_versions = requested_versions,
212213
)
214+
if result:
215+
_store_facts(facts, facts_version, index_url, result)
216+
return result
213217

214218
known_sources = {}
215219

tests/pypi/pypi_cache/pypi_cache_tests.bzl

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ def _cache(env, **kwargs):
2020
if not value:
2121
return env.expect.that_str(value)
2222

23+
if type(value) == "dict":
24+
return env.expect.that_dict(value)
25+
2326
return env.expect.that_struct(
2427
value,
2528
attrs = attrs,
@@ -266,6 +269,101 @@ def _test_pypi_cache_reads_from_facts(env):
266269

267270
_tests.append(_test_pypi_cache_reads_from_facts)
268271

272+
def _test_memory_cache_index_urls(env):
273+
"""Verifies that the cache returns stored values for index_urls."""
274+
store = {}
275+
cache = _cache(env, mctx = None, store = store)
276+
277+
fake_result = {
278+
"pkg-a": "https://pypi.org/simple/pkg-a/",
279+
"pkg_b": "https://pypi.org/simple/pkg-b/",
280+
}
281+
282+
key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None, "pkg_b": None})
283+
284+
cache.setdefault(key, fake_result)
285+
286+
got = cache.get(key)
287+
got.contains_exactly(fake_result)
288+
289+
key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None})
290+
got = cache.get(key)
291+
got.contains_exactly({"pkg-a": "https://pypi.org/simple/pkg-a/"})
292+
293+
key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-c": None})
294+
cache.get(key).equals(None)
295+
296+
_tests.append(_test_memory_cache_index_urls)
297+
298+
def _test_pypi_cache_writes_index_urls_to_facts(env):
299+
"""Verifies that setting index_urls in the cache also populates the facts store."""
300+
mock_ctx = mocks.mctx(facts = {})
301+
cache = _cache(env, mctx = mock_ctx)
302+
303+
fake_result = {
304+
"pkg-a": "https://pypi.org/simple/pkg-a/",
305+
"pkg_b": "https://pypi.org/simple/pkg-b/",
306+
}
307+
308+
key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None})
309+
310+
cache.setdefault(key, fake_result)
311+
312+
cache.get_facts().contains_exactly({
313+
"fact_version": "v1",
314+
"index_urls": {
315+
"https://pypi.org/simple/": {
316+
"pkg-a": "https://pypi.org/simple/pkg-a/",
317+
},
318+
},
319+
})
320+
321+
key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg_b": None})
322+
cache.setdefault(key, fake_result)
323+
324+
cache.get_facts().contains_exactly({
325+
"fact_version": "v1",
326+
"index_urls": {
327+
"https://pypi.org/simple/": {
328+
"pkg-a": "https://pypi.org/simple/pkg-a/",
329+
"pkg_b": "https://pypi.org/simple/pkg-b/",
330+
},
331+
},
332+
})
333+
334+
_tests.append(_test_pypi_cache_writes_index_urls_to_facts)
335+
336+
def _test_pypi_cache_reads_index_urls_from_facts(env):
337+
"""Verifies that reading index_urls from facts works correctly."""
338+
mock_ctx = mocks.mctx(facts = {
339+
"fact_version": "v1",
340+
"index_urls": {
341+
"https://pypi.org/simple/": {
342+
"pkg-a": "https://pypi.org/simple/pkg-a/",
343+
"pkg-b": "https://pypi.org/simple/pkg-b/",
344+
},
345+
},
346+
})
347+
cache = _cache(env, mctx = mock_ctx)
348+
349+
key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None})
350+
got = cache.get(key)
351+
got.contains_exactly({"pkg-a": "https://pypi.org/simple/pkg-a/"})
352+
353+
key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-a": None, "pkg-b": None})
354+
got = cache.get(key)
355+
got.contains_exactly({
356+
"pkg-a": "https://pypi.org/simple/pkg-a/",
357+
"pkg-b": "https://pypi.org/simple/pkg-b/",
358+
})
359+
360+
key = ("https://pypi.org/simple/", "https://pypi.org/simple/", {"pkg-c": None})
361+
cache.get(key).equals(None)
362+
363+
cache.get_facts().contains_exactly(mock_ctx.facts)
364+
365+
_tests.append(_test_pypi_cache_reads_index_urls_from_facts)
366+
269367
def pypi_cache_test_suite(name):
270368
test_suite(
271369
name = name,

0 commit comments

Comments
 (0)