azure-sdk-for-python icon indicating copy to clipboard operation
azure-sdk-for-python copied to clipboard

Eliminate Remaining `requests` usage within `azure-sdk-tools`

Open scbedd opened this issue 3 years ago • 2 comments

Resolves #26925

scbedd avatar Oct 27 '22 00:10 scbedd

Should we also remove the requests imports in proxy_startup.py and proxy_testcase.py?

Walter White Gif

scbedd avatar Oct 27 '22 19:10 scbedd

API change check

API changes are not detected in this pull request.

azure-sdk avatar Jan 04 '23 01:01 azure-sdk

Hey @scbedd. Ran into some issues with requests not being available in certain pipeline environments (example), and that led me here, which should resolve that issue. However, after running playback tests on a service using this PR (to test it), I ran into the following issue:

../../../tools/azure-sdk-tools/devtools_testutils/proxy_startup.py:193: in start_test_proxy
    set_custom_default_matcher(excluded_headers=headers_to_ignore)
../../../tools/azure-sdk-tools/devtools_testutils/sanitizers.py:103: in set_custom_default_matcher
    _send_matcher_request("CustomDefaultMatcher", {"x-recording-id": x_recording_id}, request_args)
../../../tools/azure-sdk-tools/devtools_testutils/sanitizers.py:563: in _send_matcher_request
    http_client.request(method="POST", url=f"{PROXY_URL}/Admin/SetMatcher", headers=headers_to_send, fields=parameters)
.tox/whl/lib/python3.10/site-packages/urllib3/request.py:78: in request
    return self.request_encode_body(
.tox/whl/lib/python3.10/site-packages/urllib3/request.py:170: in request_encode_body
    return self.urlopen(method, url, **extra_kw)
.tox/whl/lib/python3.10/site-packages/urllib3/poolmanager.py:376: in urlopen
    response = conn.urlopen(method, u.request_uri, **kw)
.tox/whl/lib/python3.10/site-packages/urllib3/connectionpool.py:702: in urlopen
    httplib_response = self._make_request(
.tox/whl/lib/python3.10/site-packages/urllib3/connectionpool.py:398: in _make_request
    conn.request(method, url, **httplib_request_kw)
.tox/whl/lib/python3.10/site-packages/urllib3/connection.py:241: in request
    super(HTTPConnection, self).request(method, url, body=body, headers=headers)
/home/pvaneck/.pyenv/versions/3.10.6/lib/python3.10/http/client.py:1282: in request
    self._send_request(method, url, body, headers, encode_chunked)
/home/pvaneck/.pyenv/versions/3.10.6/lib/python3.10/http/client.py:1323: in _send_request
    self.putheader(hdr, value)
.tox/whl/lib/python3.10/site-packages/urllib3/connection.py:226: in putheader
    _HTTPConnection.putheader(self, header, *values)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <urllib3.connection.HTTPSConnection object at 0x7f936ac6aa40>, header = b'x-recording-id', values = [None], i = 0, one_value = None

    def putheader(self, header, *values):
        """Send a request header line to the server.

        For example: h.putheader('Accept', 'text/html')
        """
        if self.__state != _CS_REQ_STARTED:
            raise CannotSendHeader()

        if hasattr(header, 'encode'):
            header = header.encode('ascii')

        if not _is_legal_header_name(header):
            raise ValueError('Invalid header name %r' % (header,))

        values = list(values)
        for i, one_value in enumerate(values):
            if hasattr(one_value, 'encode'):
                values[i] = one_value.encode('latin-1')
            elif isinstance(one_value, int):
                values[i] = str(one_value).encode('ascii')

>           if _is_illegal_header_value(values[i]):
E           TypeError: expected string or bytes-like object

After a little bit of debugging, looks like the error stems from a None value for a header being sent here where the headers are {'x-abstraction-identifier': 'CustomDefaultMatcher', 'x-recording-id': None}. Seems like requests drops the None header at some point which is why the request works with requests, but this None value is causing issues when using urllib3 directly.

Should we ensure that a recording_id is set so that the header is not None (or just not send this header if the recording_id is None)?

Absolutely! Thanks for your write-ups here Paul, effectively all of your suggestions made it into the PR in some fashion. Lack of recording id is fine, as then something will apply at the session level. I've added a filter before the abstractions to sending a request to eliminate any headers that are None.

scbedd avatar Jan 20 '23 18:01 scbedd

/azp run python - core - ci

scbedd avatar Jan 20 '23 18:01 scbedd

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Jan 20 '23 18:01 azure-pipelines[bot]