Added support for passing 'extra_headers' to Loader, AsyncImage supports using custom headers for remote paths

[unwize]

Maintainer merge checklist

• [ ] Title is descriptive/clear for inclusion in release notes.
• [ ] Applied a Component: xxx label.
• [ ] Applied the api-deprecation or api-break label.
• [ ] Applied the release-highlight label to be highlighted in release notes.
• [ ] Added to the milestone version it was merged into.
• [ ] Unittests are included in PR.
• [ ] Properly documented, including versionadded, versionchanged as needed.

[welcome[bot]]

Thanks for opening your first pull request here! 💖 Please check out our contributing guidelines.

[unwize]

Thanks, this is my first time contributing to a public project. I'd be happy to write some unit tests. Is there a specific template you'd like me to follow?

Further, what should I test specifically? whether or not the arguments get passed correctly? Should I be testing whether adding headers is functional as a whole?

Thanks for your assistance.

[tshirtman]

Thanks, this is my first time contributing to a public project. I'd be happy to write some unit tests. Is there a specific template you'd like me to follow?

There is an AsyncImageTestCase in kivy/tests/test_uix_asyncimage.py, you could add a method to that.

Further, what should I test specifically? whether or not the arguments get passed correctly? Should I be testing whether adding headers is functional as a whole?

I think using unitest.mock.patchas a contextmanager (with) to replace urllib3 and check if the request object is used as expected, something like.

from unittest.mock import patch

with patch('kivy.loader.urllib') as mock:
    image = …
    mock.request.Request.return_value.add_header.assert_called_with(...)

though it might not work right away because of the PY2/PY3 code to import urllib before, but now we dropped support for python2 you could remove that and just import urllib normally at the top of the module.

Thanks :)

[actionchains]

How to close the kivy Information I don't want to see the Information please help me

[unwize]

from unittest.mock import patch

with patch('kivy.loader.urllib') as mock:
    image = …
    mock.request.Request.return_value.add_header.assert_called_with(...)

I went ahead and added the test. I had no idea that such a useful library for testing exists!

[tshirtman]

Good, now it just needs some reformating for pep8, and it should be good :)

[unwize]

Good, now it just needs some reformating for pep8, and it should be good :)

I've been looking through my changes and can't find pep8 errors, was there a check that found some? If not, what tool can I use to identify my issues? I am currently using Pycharm and thought it was already doing PEP8 checks.

[tshirtman]

in the checks that are displayed under this conversation, there are a few that are failing, if you click on the details of the pep8 one, you can see the following log https://github.com/kivy/kivy/runs/4026271999?check_suite_focus=true

python3 -m flake8 .
./kivy/loader.py:342:81: E501 line too long (81 > 80 characters)
                    for header, value in kwargs.get("extra_headers", {}).items():
                                                                                ^
./kivy/uix/image.py:363:81: E501 line too long (110 > 80 characters)
    '''If this property is not set to None, any remote requests made will contain the headers stored in this dict.

    :attr:`extra_headers` is a :class:`~kivy.properties.DictProperty` and defaults
    to None.
    .. versionadded:: 2.1.0 
    '''
                                                                                ^
./kivy/uix/image.py:365:81: E501 line too long (82 > 80 characters)
    '''If this property is not set to None, any remote requests made will contain the headers stored in this dict.

    :attr:`extra_headers` is a :class:`~kivy.properties.DictProperty` and defaults
    to None.
    .. versionadded:: 2.1.0 
    '''
                                                                                ^
./kivy/uix/image.py:367:28: W291 trailing whitespace
    '''If this property is not set to None, any remote requests made will contain the headers stored in this dict.

    :attr:`extra_headers` is a :class:`~kivy.properties.DictProperty` and defaults
    to None.
    .. versionadded:: 2.1.0 
    '''
                           ^
./kivy/tests/test_uix_asyncimage.py:137:81: E501 line too long (106 > 80 characters)
            mock.request.Request.return_value.add_header.assert_called_with("Referer", headers["Referer"])
                                                                                ^
4     E501 line too long (81 > 80 characters)
1     W291 trailing whitespace
5

which shows both the command (python3 -m flake8) and the places where it fails. Basically a bunch of lines that are a bit too long, and one which has a trailing space at the end.

edit: the other checks seem to fail because they now try to install kivy on python3.10, which we are not ready for, not sure how that happened, i had a PR to switch to it, but i don't think it's been merged?

[unwize]

in the checks that are displayed under this conversation, there are a few that are failing, if you click on the details of the pep8 one, you can see the following log https://github.com/kivy/kivy/runs/4026271999?check_suite_focus=true

python3 -m flake8 .
./kivy/loader.py:342:81: E501 line too long (81 > 80 characters)
                    for header, value in kwargs.get("extra_headers", {}).items():
                                                                                ^
./kivy/uix/image.py:363:81: E501 line too long (110 > 80 characters)
    '''If this property is not set to None, any remote requests made will contain the headers stored in this dict.

    :attr:`extra_headers` is a :class:`~kivy.properties.DictProperty` and defaults
    to None.
    .. versionadded:: 2.1.0 
    '''
                                                                                ^
./kivy/uix/image.py:365:81: E501 line too long (82 > 80 characters)
    '''If this property is not set to None, any remote requests made will contain the headers stored in this dict.

    :attr:`extra_headers` is a :class:`~kivy.properties.DictProperty` and defaults
    to None.
    .. versionadded:: 2.1.0 
    '''
                                                                                ^
./kivy/uix/image.py:367:28: W291 trailing whitespace
    '''If this property is not set to None, any remote requests made will contain the headers stored in this dict.

    :attr:`extra_headers` is a :class:`~kivy.properties.DictProperty` and defaults
    to None.
    .. versionadded:: 2.1.0 
    '''
                           ^
./kivy/tests/test_uix_asyncimage.py:137:81: E501 line too long (106 > 80 characters)
            mock.request.Request.return_value.add_header.assert_called_with("Referer", headers["Referer"])
                                                                                ^
4     E501 line too long (81 > 80 characters)
1     W291 trailing whitespace
5

which shows both the command (python3 -m flake8) and the places where it fails. Basically a bunch of lines that are a bit too long, and one which has a trailing space at the end.

edit: the other checks seem to fail because they now try to install kivy on python3.10, which we are not ready for, not sure how that happened, i had a PR to switch to it, but i don't think it's been merged?

It looks like the branch is passing the PEP8 checks.

What happens when the PR is approved? How long does that take to get into master, and then to dependency managers like pip?

[Julian-O]

@tshirtman @SlappedWithSilence This PR seems to have got jammed up.

Changes were requested, and submitted - but the PR still says changes requested. Can we unblock this?

[misl6]

@tshirtman @SlappedWithSilence This PR seems to have got jammed up.

Changes were requested, and submitted - but the PR still says changes requested. Can we unblock this?

Changes looks good, however we need a rebase on top of latest master, and this string should be updated (2.3.0 at the moment of writing): https://github.com/kivy/kivy/blob/3707237a538962d1ca343805272fd3caeb7e6e6e/kivy/uix/image.py#L368

[unwize]

@tshirtman @SlappedWithSilence This PR seems to have got jammed up. Changes were requested, and submitted - but the PR still says changes requested. Can we unblock this?

Changes looks good, however we need a rebase on top of latest master, and this string should be updated (2.3.0 at the moment of writing):

https://github.com/kivy/kivy/blob/3707237a538962d1ca343805272fd3caeb7e6e6e/kivy/uix/image.py#L368

Gotcha. It will take me a moment to re-familiarize myself with this codebase given the two year gap, haha. I'll get on the rebase soon.

[misl6]

@tshirtman @SlappedWithSilence This PR seems to have got jammed up. Changes were requested, and submitted - but the PR still says changes requested. Can we unblock this?

Changes looks good, however we need a rebase on top of latest master, and this string should be updated (2.3.0 at the moment of writing): https://github.com/kivy/kivy/blob/3707237a538962d1ca343805272fd3caeb7e6e6e/kivy/uix/image.py#L368

Gotcha. It will take me a moment to re-familiarize myself with this codebase given the two year gap, haha. I'll get on the rebase soon.

No worries, our fault. Please ping me when you're ready.