Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Improve Exception Handling inFile.download_*#4542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Bibo-Joshi merged 9 commits intomasterfromstabilize-file-download
Nov 3, 2024

Conversation

Bibo-Joshi
Copy link
Member

@Bibo-JoshiBibo-Joshi commentedOct 31, 2024
edited
Loading

Response to the test failures on masterhttps://github.com/python-telegram-bot/python-telegram-bot/actions/runs/11607799025/job/32340491289

These are caused by Telegram failing to return afile_path in theget_file response. I'm not sure why they stopped doing that. Further investigation might be necessary. I realized (maybe for the first time) thatfile_path is actually documented to be optional.

What this PR does is mostly soften the blow, i.e. provide understandable error handling.
This will not immediately fix the test suite. I'd like to wait a moment and see if the issue solves itself. If it does not, we'll have to adjust the other tests/mark them as xfail/report to telegram.

PS: see alsotdlib/telegram-bot-api#658

@codecovCodecov
Copy link

codecovbot commentedOct 31, 2024
edited
Loading

❌ 6 Tests Failed:

Tests completedFailedPassedSkipped
599765991265
View the top 3 failed tests by shortest run time
tests._files.test_file.TestFileWithoutRequest test_download_bytearray
Stack Traces | 0.002s run time
self = <tests._files.test_file.TestFileWithoutRequest object at 0x000001BA36564D70>monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x000001BA3AAB63C0>file = File(file_id='NOTVALIDDOESNOTMATTER', file_size=28232, file_unique_id='adc3145fd2e84d95b64d68eaa22aa33e')    async def test_download_bytearray(self, monkeypatch, file):        async def test(*args, **kwargs):            return self.file_content            monkeypatch.setattr(file.get_bot().request, "retrieve", test)            # Check that a download to a newly allocated bytearray works.>       buf = await file.download_as_bytearray()tests\_files\test_file.py:217: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _self = File(file_id='NOTVALIDDOESNOTMATTER', file_size=28232, file_unique_id='adc3145fd2e84d95b64d68eaa22aa33e')buf = None    async def download_as_bytearray(        self,        buf: Optional[bytearray] = None,        *,        read_timeout: ODVInput[float] = DEFAULT_NONE,        write_timeout: ODVInput[float] = DEFAULT_NONE,        connect_timeout: ODVInput[float] = DEFAULT_NONE,        pool_timeout: ODVInput[float] = DEFAULT_NONE,    ) -> bytearray:        """Download this file and return it as a bytearray.            .. versionchanged:: NEXT.VERSION            Raises :exc:`RuntimeError` if :attr:`file_path` is not set.            Args:            buf (:obj:`bytearray`, optional): Extend the given bytearray with the downloaded data.            Keyword Args:            read_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.read_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.                    .. versionadded:: 20.0            write_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.write_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.                    .. versionadded:: 20.0            connect_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.connect_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.                    .. versionadded:: 20.0            pool_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.pool_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.                    .. versionadded:: 20.0            Returns:            :obj:`bytearray`: The same object as :paramref:`buf` if it was specified. Otherwise a            newly allocated :obj:`bytearray`.            Raises:            RuntimeError: If :attr:`file_path` is not set.            """        if not self.file_path:>           raise RuntimeError("No `file_path` available for this file. Can not download.")E           RuntimeError: No `file_path` available for this file. Can not download.telegram\_files\file.py:341: RuntimeError
tests._files.test_file.TestFileWithoutRequest test_download_no_filename
Stack Traces | 0.002s run time
self = <tests._files.test_file.TestFileWithoutRequest object at 0x000001BA363CB450>monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x000001BA3AAB7A80>file = File(file_id='NOTVALIDDOESNOTMATTER', file_size=28232, file_unique_id='adc3145fd2e84d95b64d68eaa22aa33e')    async def test_download_no_filename(self, monkeypatch, file):        async def test(*args, **kwargs):            return self.file_content            file.file_path = None            monkeypatch.setattr(file.get_bot().request, "retrieve", test)>       out_file = await file.download_to_drive()tests\_files\test_file.py:192: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _self = File(file_id='NOTVALIDDOESNOTMATTER', file_size=28232, file_unique_id='adc3145fd2e84d95b64d68eaa22aa33e')custom_path = None    async def download_to_drive(        self,        custom_path: Optional[FilePathInput] = None,        *,        read_timeout: ODVInput[float] = DEFAULT_NONE,        write_timeout: ODVInput[float] = DEFAULT_NONE,        connect_timeout: ODVInput[float] = DEFAULT_NONE,        pool_timeout: ODVInput[float] = DEFAULT_NONE,    ) -> Path:        """        Download this file. By default, the file is saved in the current working directory with        :attr:`file_path` as file name. If the file has no filename, the file ID will be used as        filename. If :paramref:`custom_path` is supplied as a :obj:`str` or :obj:`pathlib.Path`,        it will be saved to that path.            Note:            If :paramref:`custom_path` isn't provided and :attr:`file_path` is the path of a            local file (which is the case when a Bot API Server is running in local mode), this            method will just return the path.                The only exception to this are encrypted files (e.g. a passport file). For these, a            file with the prefix `decrypted_` will be created in the same directory as the            original file in order to decrypt the file without changing the existing one            in-place.            .. seealso:: :wiki:`Working with Files and Media <Working-with-Files-and-Media>`            .. versionchanged:: 20.0                * :paramref:`custom_path` parameter now also accepts :class:`pathlib.Path` as argument.            * Returns :class:`pathlib.Path` object in cases where previously a :obj:`str` was              returned.            * This method was previously called ``download``. It was split into              :meth:`download_to_drive` and :meth:`download_to_memory`.            .. versionchanged:: NEXT.VERSION            Raises :exc:`RuntimeError` if :attr:`file_path` is not set.            Args:            custom_path (:class:`pathlib.Path` | :obj:`str` , optional): The path where the file                will be saved to. If not specified, will be saved in the current working directory                with :attr:`file_path` as file name or the :attr:`file_id` if :attr:`file_path`                is not set.            Keyword Args:            read_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.read_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.            write_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.write_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.            connect_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.connect_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.            pool_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.pool_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.            Returns:            :class:`pathlib.Path`: Returns the Path object the file was downloaded to.            Raises:            RuntimeError: If :attr:`file_path` is not set.            """        if not self.file_path:>           raise RuntimeError("No `file_path` available for this file. Can not download.")E           RuntimeError: No `file_path` available for this file. Can not download.telegram\_files\file.py:186: RuntimeError
tests._files.test_file.TestFileWithoutRequest test_download_file_obj
Stack Traces | 0.003s run time
self = <tests._files.test_file.TestFileWithoutRequest object at 0x000001BA363CB550>monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x000001BA3AAB5FD0>file = File(file_id='NOTVALIDDOESNOTMATTER', file_size=28232, file_unique_id='adc3145fd2e84d95b64d68eaa22aa33e')    async def test_download_file_obj(self, monkeypatch, file):        async def test(*args, **kwargs):            return self.file_content            monkeypatch.setattr(file.get_bot().request, "retrieve", test)        with TemporaryFile() as custom_fobj:>           await file.download_to_memory(out=custom_fobj)tests\_files\test_file.py:206: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _self = File(file_id='NOTVALIDDOESNOTMATTER', file_size=28232, file_unique_id='adc3145fd2e84d95b64d68eaa22aa33e')out = <tempfile._TemporaryFileWrapper object at 0x000001BA392DF610>    async def download_to_memory(        self,        out: BinaryIO,        *,        read_timeout: ODVInput[float] = DEFAULT_NONE,        write_timeout: ODVInput[float] = DEFAULT_NONE,        connect_timeout: ODVInput[float] = DEFAULT_NONE,        pool_timeout: ODVInput[float] = DEFAULT_NONE,    ) -> None:        """        Download this file into memory. :paramref:`out` needs to be supplied with a        :obj:`io.BufferedIOBase`, the file contents will be saved to that object using the        :obj:`out.write<io.BufferedIOBase.write>` method.            .. seealso:: :wiki:`Working with Files and Media <Working-with-Files-and-Media>`            Hint:            If you want to immediately read the data from ``out`` after calling this method, you            should call ``out.seek(0)`` first. See also :meth:`io.IOBase.seek`.            .. versionadded:: 20.0            .. versionchanged:: NEXT.VERSION            Raises :exc:`RuntimeError` if :attr:`file_path` is not set.            Args:            out (:obj:`io.BufferedIOBase`): A file-like object. Must be opened for writing in                binary mode.            Keyword Args:            read_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.read_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.            write_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.write_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.            connect_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.connect_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.            pool_timeout (:obj:`float` | :obj:`None`, optional): Value to pass to                :paramref:`telegram.request.BaseRequest.post.pool_timeout`. Defaults to                :attr:`~telegram.request.BaseRequest.DEFAULT_NONE`.            Raises:            RuntimeError: If :attr:`file_path` is not set.        """        if not self.file_path:>           raise RuntimeError("No `file_path` available for this file. Can not download.")E           RuntimeError: No `file_path` available for this file. Can not download.telegram\_files\file.py:274: RuntimeError

To view individual test run time comparison to the main branch, go to theTest Analytics Dashboard

@Bibo-Joshi
Copy link
MemberAuthor

Lastest commit

@Poolitzer
Copy link
Member

Well hindsight is is 20/20.

Copy link
Member

@PoolitzerPoolitzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM

@Bibo-Joshi
Copy link
MemberAuthor

New commits revert the workaround forget_file again astdlib/telegram-bot-api#658 has been resolved by now 😅

Copy link
Member

@PoolitzerPoolitzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM

@Bibo-JoshiBibo-Joshi merged commit507d6bc intomasterNov 3, 2024
22 of 23 checks passed
@Bibo-JoshiBibo-Joshi deleted the stabilize-file-download branchNovember 3, 2024 15:35
@Bibo-JoshiBibo-Joshi added 🔌 enhancementpr description: enhancement and removed enhancement labelsNov 3, 2024
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsNov 11, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@PoolitzerPoolitzerPoolitzer approved these changes

@harshil21harshil21Awaiting requested review from harshil21

Assignees
No one assigned
Labels
🔌 enhancementpr description: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@Bibo-Joshi@Poolitzer

[8]ページ先頭

©2009-2025 Movatter.jp