mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 02:48:38 -04:00
Fix swapped arguments in download_info_json Video.get_by_url call
`download_info_json` called `Video.get_by_url(url, session)`, but the method signature is `get_by_url(session, url)`. This passed the URL string where the session was expected, raising `AttributeError: 'str' object has no attribute 'query'` whenever a Video metadata refresh ran. Swap the arguments at both call sites and add a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
947ee01b25
commit
5427beb5fb
2 changed files with 25 additions and 2 deletions
|
|
@ -1187,7 +1187,7 @@ class VideoDownloader(Downloader, ABC):
|
|||
|
||||
with get_db_session() as session:
|
||||
# Find the existing video, replace its info json.
|
||||
video = Video.get_by_url(url, session)
|
||||
video = Video.get_by_url(session, url)
|
||||
|
||||
location = video.location
|
||||
|
||||
|
|
@ -1202,7 +1202,7 @@ class VideoDownloader(Downloader, ABC):
|
|||
|
||||
with get_db_session(commit=True) as session:
|
||||
# Find the existing video, replace its info json.
|
||||
video = Video.get_by_url(url, session)
|
||||
video = Video.get_by_url(session, url)
|
||||
video.replace_info_json(info_json)
|
||||
|
||||
if parent_download_url := settings.get('parent_download_url'):
|
||||
|
|
|
|||
|
|
@ -538,6 +538,29 @@ async def test_video_download(test_session, test_directory, mock_video_extract_i
|
|||
assert 'formats' not in video.get_info_json(), 'formats should be deleted from info json'
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_download_info_json_finds_existing_video(test_session, test_directory, video_factory):
|
||||
"""`download_info_json` looks up the existing Video by its URL.
|
||||
|
||||
Regression: `Video.get_by_url` was called with its arguments swapped — `(url, session)` instead of
|
||||
`(session, url)` — which raised `AttributeError: 'str' object has no attribute 'query'`."""
|
||||
source_id = 'video-source-id'
|
||||
url = 'https://example.com/the-video'
|
||||
video = video_factory(with_info_json={'id': source_id}, source_id=source_id)
|
||||
video.file_group.url = url
|
||||
test_session.commit()
|
||||
|
||||
download = Download(url=url, info_json={'id': source_id}, settings={})
|
||||
|
||||
# The info_json download itself fails here, but the buggy existing-video lookup runs first.
|
||||
with mock.patch('modules.videos.downloader.download_video_info_json') as mock_download:
|
||||
mock_download.side_effect = Exception('no network')
|
||||
result = await VideoDownloader.download_info_json(download, video.video_path)
|
||||
|
||||
assert result.success is False, 'Download should fail gracefully once the info json download fails'
|
||||
mock_download.assert_called_once_with(url)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_download_result(test_session, test_directory, video_download_manager, mock_video_process_runner,
|
||||
mock_video_extract_info, image_file, await_switches):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue