Repository navigation
Point catalog loader test fixtures at a placeholder host - #5609
Merged
Merged
Conversation
The loader TSV fixtures carry image URLs on a third-party host that has long since stopped serving them. Nothing in these tests fetches the URLs — they are only loaded into the intermediate table — so swap the host for the reserved example.com domain and keep the fixtures self-contained. Also drop the Thingiverse example-output fixture. Thingiverse ingestion was removed a while back, leaving it the only file in that directory without a matching provider script. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
obenland
requested review from
krysal and
obulat
and removed request for
a team
September 2, 2026 16:45
openverse-bot
added
🧱 stack: catalog
openverse-bot
added
the
🏷 status: label work required
There was a problem hiding this comment.
🟢 Approval recommended
The changes are limited to test fixtures and deletion of an unreferenced file, with no remaining references to the removed fixture and no functional code-path impact.
Pull request overview
This PR performs small catalog test-fixture housekeeping by removing a stale, unused Thingiverse example-output TSV and updating loader TSV fixtures to use example.com for previously hard-coded image hosts that no longer serve content, keeping the fixtures self-contained without changing test semantics.
Changes:
- Replaced
thingiverse-rerender-new.s3.amazonaws.comimage/thumbnail hosts withhttps://example.com/...in loader TSV fixtures used bycatalog/tests/dags/common/loader/test_sql.py. - Removed the leftover
thingiverse_1551879215.tsvprovider example-output fixture, which is no longer referenced.
File summaries
| File | Description |
|---|---|
| catalog/tests/dags/common/loader/test_resources/none_missing.tsv | Swaps deprecated image host to example.com while preserving paths/rows for loader tests. |
| catalog/tests/dags/common/loader/test_resources/malformed_more_than_max_rows.tsv | Updates placeholder image host in malformed fixture data. |
| catalog/tests/dags/common/loader/test_resources/malformed_max_rows.tsv | Updates placeholder image host in malformed fixture data. |
| catalog/tests/dags/common/loader/test_resources/malformed_less_than_max_rows.tsv | Updates placeholder image host in malformed fixture data. |
| catalog/tests/dags/common/loader/test_resources/license_missing.tsv | Updates placeholder image host in license-missing fixture rows. |
| catalog/tests/dags/common/loader/test_resources/foreign_landing_url_missing.tsv | Updates placeholder image host in fixture rows with missing landing URL. |
| catalog/tests/dags/common/loader/test_resources/foreign_identifier_duplicate.tsv | Updates placeholder image host in duplicate-identifier fixture rows. |
| catalog/tests/dags/providers/provider_api_scripts/resources/example_output/thingiverse_1551879215.tsv | Removes unused legacy Thingiverse example output TSV. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
obenland
added
🧰 goal: internal improvement
github-project-automation
Bot
moved this from 👀 Needs Review
to 🤝 Merged
in Openverse PRs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
No issue — small catalog test housekeeping.
Description
Two related bits of tidying in the catalog test fixtures:
The loader TSV fixtures in
catalog/tests/dags/common/loader/test_resources/carry
url/thumbnailvalues on a third-party host that stopped serving thoseimages a long time ago.
test_sql.pyonly copies these rows into theintermediate table — it never fetches them — so the host is swapped for the
reserved
example.comdomain and the fixtures become self-contained. Paths,row counts, and column counts are unchanged, so every uniqueness and
null-handling case the tests rely on still behaves the same way.
catalog/tests/dags/.../resources/example_output/thingiverse_1551879215.tsvisremoved. Thingiverse ingestion was dropped some time ago and this was the only
file left in that directory without a matching provider script; nothing
references it.
Testing Instructions
ov just catalog/test catalog/tests/dags/common/loader/test_sql.py— the loadertests should pass unchanged.
Checklist
Update index.md).main) or a parent feature branch.ov just catalog/generate-docsfor catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-propsfor the catalog or
ov just api/generate-docsfor the API) where applicable. — n/a, no DAG or media-property changes🤖 Generated with Claude Code