Cleanup lingeting t_thumb in cover and screenshto art #1940
Conversation
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
|
Running Code Quality on PRs by uploading data to Trunk will soon be removed. You can still run checks on your PRs using trunk-action - see the migration guide for more information. |
There was a problem hiding this comment.
Pull Request Overview
This PR removes lingering t_thumb suffixes from cover and screenshot URLs, refactors RA metadata assignment in the socket-based scan endpoint, and drops commented-out background scan code.
- Adds an Alembic migration to replace
t_thumbwitht_1080pinurl_coverandt_720pinurl_screenshots - Updates
_identify_romlogic to requirera_id, guardra_metadata, and convert it to a new dict - Cleans up a minor comment and removes dead code in scan handlers
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/handler/scan_handler.py | Minor blank-line removal and corrected conjunction in a comment |
| backend/endpoints/sockets/scan.py | Enforce ra_id presence, safer ra_metadata handling, removed dead code |
| backend/alembic/versions/0041_assets_t_thumb_cleanup.py | Migration to clean up t_thumb suffixes in existing asset URLs |
Comments suppressed due to low confidence (1)
backend/endpoints/sockets/scan.py:506
- The new
ra_metadatabranching logic isn't covered by existing tests. Add unit tests for cases wherera_metadataisNoneand where it contains anachievementsarray to verify both paths.
ra_metadata = ra_handler_rom.get("ra_metadata", None)
gantoine
commented
Jun 6, 2025
| badge_url = a.get("badge_url", None) | ||
| badge_path = a.get("badge_path", None) | ||
| if badge_url and badge_path: | ||
| await fs_resource_handler.store_badge(badge_url, badge_path) |
There was a problem hiding this comment.
This is just some defensive coding, behaviour should be the same
zurdi15
approved these changes
Jun 6, 2025
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.
Description
Explain the changes or enhancements you are proposing with this pull request.
Lingering
t_thumbvalues inurl_coverandurl_screenshotswill cause romm to overrite cover and screenshot art with thumbnails on partial scans. This replaces all uses oft_thumbin those fields witht_1080pfor covers andt_720pfor screenshots. Tested on both mariadb and postgres locally.Some
cover_urlfields inigdb_metadatamight still uset_thumband would still be affected, but there's no easy way top clean those up through SQL.Checklist
Please check all that apply.
Screenshots