Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_url_to_node_config[https://[::1]:0/-https://[::1]:0-] fails with latest release of urllib3 (urllib3==1.26.13) #96

Open
danigm opened this issue Dec 5, 2022 · 1 comment

Comments

@danigm
Copy link

danigm commented Dec 5, 2022

The last version of urllib3, 1.26.13 parses differently the port 0 in urls and that breaks the tests:

https://github.com/urllib3/urllib3/releases/tag/1.26.13

This is the test that's failing:

_________________________________________________________ test_url_to_node_config[https://[::1]:0/-https://[::1]:0-] __________________________________________________________

url = 'https://[::1]:0/', node_base_url = 'https://[::1]:0', path_prefix = ''

    @pytest.mark.parametrize(
        ["url", "node_base_url", "path_prefix"],
        [
            ("http://localhost:3002", "http://localhost:3002", ""),
            ("http://127.0.0.1:3002", "http://127.0.0.1:3002", ""),
            ("http://127.0.0.1:3002/", "http://127.0.0.1:3002", ""),
            (
                "http://127.0.0.1:3002/path-prefix",
                "http://127.0.0.1:3002/path-prefix",
                "/path-prefix",
            ),
            (
                "http://localhost:3002/url-prefix/",
                "http://localhost:3002/url-prefix",
                "/url-prefix",
            ),
            ("http://[::1]:3002/url-prefix", "http://[::1]:3002/url-prefix", "/url-prefix"),
            ("https://[::1]:0/", "https://[::1]:0", ""),
        ],
    )
    def test_url_to_node_config(url, node_base_url, path_prefix):
>       node_config = url_to_node_config(url)

tests/test_client_utils.py:166: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

url = 'https://[::1]:0/', use_default_ports_for_scheme = False

    def url_to_node_config(
        url: str, use_default_ports_for_scheme: bool = False
    ) -> NodeConfig:
        """Constructs a :class:`elastic_transport.NodeConfig` instance from a URL.
        If a username/password are specified in the URL they are converted to an
        'Authorization' header.
    
        :param url: URL to transform into a NodeConfig.
        :param use_default_ports_for_scheme: If 'True' will resolve default ports for the given scheme.
        """
        try:
            parsed_url = parse_url(url)
        except LocationParseError:
            raise ValueError(f"Could not parse URL {url!r}") from None
    
        # Only fill in a default port for HTTP and HTTPS
        # when we know the scheme is one of those two.
        parsed_port: Optional[int] = parsed_url.port  # type: ignore[assignment]
        if (
            parsed_url.port is None
            and parsed_url.scheme is not None
            and use_default_ports_for_scheme is True
        ):
            if parsed_url.scheme == "https":
                parsed_port = 443
            elif parsed_url.scheme == "http":
                parsed_port = 80
    
        if any(
            component in (None, "")
            for component in (parsed_url.scheme, parsed_url.host, parsed_port)
        ):
>           raise ValueError(
                "URL must include a 'scheme', 'host', and 'port' component (ie 'https://localhost:9200')"
            )
E           ValueError: URL must include a 'scheme', 'host', and 'port' component (ie 'https://localhost:9200')

danigm added a commit to danigm/elastic-transport-python that referenced this issue Dec 5, 2022
Removes the broken url test with latest urllib3 release.

The 1.26.13 urllib3 release removes all the leading zero in port parsing
so the zero port is similar to do not provide the port:
https://github.com/urllib3/urllib3/releases/tag/1.26.13

Fix elastic#96
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this issue Dec 7, 2022
@pquentin
Copy link
Member

This is a bug that will get fixed in urllib3 1.26.14. There's no release date yet though, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants