Calculate hashes before fetching metadata #1981
Conversation
|
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. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This pull request adjusts the order of hash calculations so that hashes are computed before scanning metadata, while also updating and refactoring several related functions. Key changes include:
- Moving hash calculations earlier in the scan process and updating related function signatures.
- Renaming and refactoring functions for clarity (e.g. renaming missing_rom_files to purge_rom_files).
- Updating file handling and response handling in tests, metadata, and filesystem modules.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| backend/models/rom.py | Modified multi-game detection comment for clarity |
| backend/handler/tests/test_fastapi.py | Updated test parameters to align with new scan_rom function signature |
| backend/handler/scan_handler.py | Reordered arguments and updated condition checks for metadata fetching |
| backend/handler/metadata/ra_handler.py | Refactored _search_rom to avoid variable shadowing |
| backend/handler/filesystem/tests/test_fs.py | Updated tests for asynchronous get_roms function |
| backend/handler/filesystem/roms_handler.py | Updated _build_rom_file and get_rom_files to include hash info in ROM files |
| backend/handler/filesystem/resources_handler.py | Added functions to store retroachievements badges |
| backend/handler/database/roms_handler.py | Renamed and refactored function to purge rom files instead of marking them missing |
| backend/endpoints/sockets/scan.py | Minor cleanups and added type: ignore in socket events |
| backend/adapters/services/rahasher.py | Moved RA_ID_TO_SLUG import inside the function for better encapsulation |
Comments suppressed due to low confidence (2)
backend/handler/metadata/ra_handler.py:193
- [nitpick] The loop variable 'r' is not very descriptive; consider renaming it to something like 'rom_item' to improve readability.
for r in roms:
backend/handler/filesystem/roms_handler.py:317
- [nitpick] Consider adding a docstring or inline comment that documents the structure and order of the returned tuple (rom_files, crc_hash, md5_hash, sha1_hash, ra_hash) for better clarity.
async def get_rom_files(self, rom: Rom) -> tuple[list[RomFile], str, str, str, str]:
| @@ -787,16 +787,15 @@ def update_rom_file(self, id: int, data: dict, session: Session = None) -> RomFi | |||
| return session.query(RomFile).filter_by(id=id).one() | |||
|
|
|||
| @begin_session | |||
| def missing_rom_files( | |||
| def purge_rom_files( | |||
There was a problem hiding this comment.
Is this change expected, or a bad rebase after Zurdi's changes?
There was a problem hiding this comment.
it's expected, we want to purge missing files since a) they dont have metadata and b) we can rescan to repopulate them
Description
Explain the changes or enhancements you are proposing with this pull request.
This PR moves the calculation of hashes before metadata scans. While this does have an impact on performance, it's a requirement for implementing playmatch and hasheous, and brings RA matching more in line with our other matchers.
Checklist
Please check all that apply.