{ACR} Add connection pooling with ACR registries. #30520

oxpa · 2024-12-15T19:33:21Z

Related command
az acr repository list

Description
I have a registry of 60000 (sixty thousands) images. Listing them usually takes ~10 minutes on my internet connection. Largely because listing happens in pages by 100 entries per page. So az cli does ~600 requests to list all images. Each request uses it's own connection.
With this change a request to the same registry will reuse an existing connection (if any) or open a new one.
Connection pooling cuts time from 10+ minutes for listing to under a minute (for my specific case). So at least 60 times better.
It should not break things as I make no initial configuration for a session so no parameters are reused. But I'm happy to fix my code if this naïve approach doesn't work.

Testing Guide
az acr repository list --debug --name some_very_large_repository
Before the patch, every request to a registry would produce a message like this:

urllib3.connectionpool: Starting new HTTPS connection ......

With the patch the message goes away, only the first request creates a connection.
The message still there for authentication and other requests.

This checklist is used to make sure that common guidelines for a pull request are followed.

The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.

azure-client-tools-bot-prd · 2024-12-15T19:33:24Z

❌AzureCLI-FullTest

❌acr

❌2020-09-01-hybrid

❌3.12

Type Test Case Error Message Line

Failed test_helm_delete self = <azure.cli.command_modules.acr.tests.hybrid_2020_09_01.test_acr_commands_mock.AcrMockCommandsTests testMethod=test_helm_delete>
mock_requests_get = <function request at 0x7f13f4a5cb80>
mock_get_access_credentials = <function get_access_credentials at 0x7f13f4a5d1c0>

    @mock.patch('azure.cli.command_modules.acr.helm.get_access_credentials', autospec=True)
    @mock.patch('requests.request', autospec=True)
    def test_helm_delete(self, mock_requests_get, mock_get_access_credentials):
        cmd = self.setup_cmd()

        response = mock.MagicMock()
        response.headers = {}
        response.status_code = 200
        mock_requests_get.return_value = response

        mock_get_access_credentials.return_value = 'testregistry.azurecr.io', EMPTY_GUID, 'password'

        # Delete all versions of a chart
>       acr_helm_delete(cmd, 'testregistry', 'mychart1', repository='testrepository', yes=True)

src/azure-cli/azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:609:

src/azure-cli/azure/cli/command_modules/acr/helm.py:103: in acr_helm_delete
    return request_data_from_registry(
                                       _

http_method = 'delete', login_server = 'testregistry.azurecr.io'
path = '/helm/v1/testrepository/_charts/mychart1'
username = '00000000-0000-0000-0000-000000000000', password = 'password'
result_index = None, json_payload = None, file_payload = None, params = None
manifest_headers = False, raw = False, retry_times = 3, retry_interval = 5
timeout = 300

    def request_data_from_registry(http_method,
                                   login_server,
                                   path,
                                   username,
                                   password,
                                   result_index=None,
                                   json_payload=None,
                                   file_payload=None,
                                   params=None,
                                   manifest_headers=False,
                                   raw=False,
                                   retry_times=3,
                                   retry_interval=5,
                                   timeout=300):
        if http_method not in ALLOWED_HTTP_METHOD:
            raise ValueError("Allowed http method: {}".format(ALLOWED_HTTP_METHOD))

        if json_payload and file_payload:
            raise ValueError("One of json_payload and file_payload can be specified.")

        if http_method in ['get', 'delete'] and (json_payload or file_payload):
            raise ValueError("Empty payload is required for http method: {}".format(http_method))

        if http_method in ['patch', 'put'] and not (json_payload or file_payload):
            raise ValueError("Non-empty payload is required for http method: {}".format(http_method))

        url = 'https://{}{}'.format(login_server, path)

        if manifest_headers:
            headers = get_manifest_authorization_header(username, password)
        else:
            headers = get_authorization_header(username, password)

        for i in range(0, retry_times):
            errorMessage = None
            try:
                if file_payload:
                    with open(file_payload, 'rb') as data_payload:
                        logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                        response = session.request(
                            method=http_method,
                            url=url,
                            headers=headers,
                            params=params,
                            data=data_payload,
                            timeout=timeout,
                            verify=(not should_disable_connection_verify())
                        )
                else:
                    logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                    response = session.request(
                        method=http_method,
                        url=url,
                        headers=headers,
                        params=params,
                        json=json_payload,
                        timeout=timeout,
                        verify=(not should_disable_connection_verify())
                    )

                log_registry_response(response)

                if manifest_headers and raw and response.status_code == 200:
                    return response.content.decode('utf-8'), None, response.status_code
                if response.status_code == 200:
                    result = response.json()[result_index] if result_index else response.json()
                    next_link = response.headers['link'] if 'link' in response.headers else None
                    return result, next_link, response.status_code
                if response.status_code == 201 or response.status_code == 202:
                    result = None
                    try:
                        result = response.json()[result_index] if result_index else response.json()
                    except ValueError as e:
                        logger.debug('Response is empty or is not a valid json. Exception: %s', str(e))
                    return result, None, response.status_code
                if response.status_code == 204:
                    return None, None, response.status_code
                if response.status_code == 401:
>                   raise RegistryException(
                        parse_error_message('Authentication required.', response),
                        response.status_code)
E                       azure.cli.command_modules.acr._docker_utils.RegistryException: 2025-01-26 02:33:48.305521 Error: authentication required, visit https://aka.ms/acr/authorization for more information. Correlation ID: ca05429e-c9b1-4cd1-81fa-35df8aca0ecc.

src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py:637: RegistryException azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:595

Failed test_helm_list self = <azure.cli.command_modules.acr.tests.hybrid_2020_09_01.test_acr_commands_mock.AcrMockCommandsTests testMethod=test_helm_list>
mock_requests_get = <function request at 0x7f13f4a5ce00>
mock_get_access_credentials = <function get_access_credentials at 0x7f13f4a5d440>

    @mock.patch('azure.cli.command_modules.acr.helm.get_access_credentials', autospec=True)
    @mock.patch('requests.request', autospec=True)
    def test_helm_list(self, mock_requests_get, mock_get_access_credentials):
        cmd = self.setup_cmd()

        response = mock.MagicMock()
        response.headers = {}
        response.status_code = 200
        response.content = json.dumps({
            'mychart1': [
                {
                    'name': 'mychart1',
                    'version': '0.2.1'
                },
                {
                    'name': 'mychart1',
                    'version': '0.1.2'
                }
            ],
            'mychart2': [
                {
                    'name': 'mychart2',
                    'version': '2.1.0'
                }
            ]}).encode()
        mock_requests_get.return_value = response

        mock_get_access_credentials.return_value = 'testregistry.azurecr.io', EMPTY_GUID, 'password'
>       acr_helm_list(cmd, 'testregistry', repository='testrepository')

src/azure-cli/azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:541:

src/azure-cli/azure/cli/command_modules/acr/helm.py:42: in acr_helm_list
    return request_data_from_registry(
                                       _

http_method = 'get', login_server = 'testregistry.azurecr.io'
path = '/helm/v1/testrepository/_charts'
username = '00000000-0000-0000-0000-000000000000', password = 'password'
result_index = None, json_payload = None, file_payload = None, params = None
manifest_headers = False, raw = False, retry_times = 3, retry_interval = 5
timeout = 300

    def request_data_from_registry(http_method,
                                   login_server,
                                   path,
                                   username,
                                   password,
                                   result_index=None,
                                   json_payload=None,
                                   file_payload=None,
                                   params=None,
                                   manifest_headers=False,
                                   raw=False,
                                   retry_times=3,
                                   retry_interval=5,
                                   timeout=300):
        if http_method not in ALLOWED_HTTP_METHOD:
            raise ValueError("Allowed http method: {}".format(ALLOWED_HTTP_METHOD))

        if json_payload and file_payload:
            raise ValueError("One of json_payload and file_payload can be specified.")

        if http_method in ['get', 'delete'] and (json_payload or file_payload):
            raise ValueError("Empty payload is required for http method: {}".format(http_method))

        if http_method in ['patch', 'put'] and not (json_payload or file_payload):
            raise ValueError("Non-empty payload is required for http method: {}".format(http_method))

        url = 'https://{}{}'.format(login_server, path)

        if manifest_headers:
            headers = get_manifest_authorization_header(username, password)
        else:
            headers = get_authorization_header(username, password)

        for i in range(0, retry_times):
            errorMessage = None
            try:
                if file_payload:
                    with open(file_payload, 'rb') as data_payload:
                        logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                        response = session.request(
                            method=http_method,
                            url=url,
                            headers=headers,
                            params=params,
                            data=data_payload,
                            timeout=timeout,
                            verify=(not should_disable_connection_verify())
                        )
                else:
                    logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                    response = session.request(
                        method=http_method,
                        url=url,
                        headers=headers,
                        params=params,
                        json=json_payload,
                        timeout=timeout,
                        verify=(not should_disable_connection_verify())
                    )

                log_registry_response(response)

                if manifest_headers and raw and response.status_code == 200:
                    return response.content.decode('utf-8'), None, response.status_code
                if response.status_code == 200:
                    result = response.json()[result_index] if result_index else response.json()
                    next_link = response.headers['link'] if 'link' in response.headers else None
                    return result, next_link, response.status_code
                if response.status_code == 201 or response.status_code == 202:
                    result = None
                    try:
                        result = response.json()[result_index] if result_index else response.json()
                    except ValueError as e:
                        logger.debug('Response is empty or is not a valid json. Exception: %s', str(e))
                    return result, None, response.status_code
                if response.status_code == 204:
                    return None, None, response.status_code
                if response.status_code == 401:
>                   raise RegistryException(
                        parse_error_message('Authentication required.', response),
                        response.status_code)
E                       azure.cli.command_modules.acr._docker_utils.RegistryException: 2025-01-26 02:33:48.749787 Error: authentication required, visit https://aka.ms/acr/authorization for more information. Correlation ID: fd3743f9-07f7-4574-9a4d-a65f85d3093a.

src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py:637: RegistryException azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:512

Failed test_helm_push self = <azure.cli.command_modules.acr.tests.hybrid_2020_09_01.test_acr_commands_mock.AcrMockCommandsTests testMethod=test_helm_push>
mock_requests_get = <function request at 0x7f13f4a5d080>
mock_get_access_credentials = <function get_access_credentials at 0x7f13f4a5d6c0>

    @mock.patch('azure.cli.command_modules.acr.helm.get_access_credentials', autospec=True)
    @mock.patch('requests.request', autospec=True)
    def test_helm_push(self, mock_requests_get, mock_get_access_credentials):
        cmd = self.setup_cmd()

        response = mock.MagicMock()
        response.headers = {}
        response.status_code = 200
        mock_requests_get.return_value = response

        mock_get_access_credentials.return_value = 'testregistry.azurecr.io', EMPTY_GUID, 'password'

        builtins_open = 'builtin.open' if sys.version_info[0] < 3 else 'builtins.open'

        # Push a chart
        with mock.patch(builtins_open) as mock_open:
            mock_open.return_value = mock.MagicMock()
>           acr_helm_push(cmd, 'testregistry', './charts/mychart1-0.2.1.tgz', repository='testrepository')

src/azure-cli/azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:647:

src/azure-cli/azure/cli/command_modules/acr/helm.py:137: in acr_helm_push
    result = request_data_from_registry(
                                       _

http_method = 'put', login_server = 'testregistry.azurecr.io'
path = '/helm/v1/testrepository/_blobs/mychart1-0.2.1.tgz'
username = '00000000-0000-0000-0000-000000000000', password = 'password'
result_index = None, json_payload = None
file_payload = './charts/mychart1-0.2.1.tgz', params = None
manifest_headers = False, raw = False, retry_times = 3, retry_interval = 5
timeout = 300

    def request_data_from_registry(http_method,
                                   login_server,
                                   path,
                                   username,
                                   password,
                                   result_index=None,
                                   json_payload=None,
                                   file_payload=None,
                                   params=None,
                                   manifest_headers=False,
                                   raw=False,
                                   retry_times=3,
                                   retry_interval=5,
                                   timeout=300):
        if http_method not in ALLOWED_HTTP_METHOD:
            raise ValueError("Allowed http method: {}".format(ALLOWED_HTTP_METHOD))

        if json_payload and file_payload:
            raise ValueError("One of json_payload and file_payload can be specified.")

        if http_method in ['get', 'delete'] and (json_payload or file_payload):
            raise ValueError("Empty payload is required for http method: {}".format(http_method))

        if http_method in ['patch', 'put'] and not (json_payload or file_payload):
            raise ValueError("Non-empty payload is required for http method: {}".format(http_method))

        url = 'https://{}{}'.format(login_server, path)

        if manifest_headers:
            headers = get_manifest_authorization_header(username, password)
        else:
            headers = get_authorization_header(username, password)

        for i in range(0, retry_times):
            errorMessage = None
            try:
                if file_payload:
                    with open(file_payload, 'rb') as data_payload:
                        logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                        response = session.request(
                            method=http_method,
                            url=url,
                            headers=headers,
                            params=params,
                            data=data_payload,
                            timeout=timeout,
                            verify=(not should_disable_connection_verify())
                        )
                else:
                    logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                    response = session.request(
                        method=http_method,
                        url=url,
                        headers=headers,
                        params=params,
                        json=json_payload,
                        timeout=timeout,
                        verify=(not should_disable_connection_verify())
                    )

                log_registry_response(response)

                if manifest_headers and raw and response.status_code == 200:
                    return response.content.decode('utf-8'), None, response.status_code
                if response.status_code == 200:
                    result = response.json()[result_index] if result_index else response.json()
                    next_link = response.headers['link'] if 'link' in response.headers else None
                    return result, next_link, response.status_code
                if response.status_code == 201 or response.status_code == 202:
                    result = None
                    try:
                        result = response.json()[result_index] if result_index else response.json()
                    except ValueError as e:
                        logger.debug('Response is empty or is not a valid json. Exception: %s', str(e))
                    return result, None, response.status_code
                if response.status_code == 204:
                    return None, None, response.status_code
                if response.status_code == 401:
>                   raise RegistryException(
                        parse_error_message('Authentication required.', response),
                        response.status_code)
E                       azure.cli.command_modules.acr._docker_utils.RegistryException: 2025-01-26 02:33:49.113961 Error: authentication required, visit https://aka.ms/acr/authorization for more information. Correlation ID: da31e10d-a11a-4b51-bda1-003aac205644.

src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py:637: RegistryException azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:629

Failed test_helm_show The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:550

Failed test_repository_delete The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:317

Failed test_repository_list The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:50

Failed test_repository_show The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:253

Failed test_repository_show_manifests The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:147

Failed test_repository_show_tags The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:91

❌3.9

Type Test Case Error Message Line

Failed test_helm_delete self = <azure.cli.command_modules.acr.tests.hybrid_2020_09_01.test_acr_commands_mock.AcrMockCommandsTests testMethod=test_helm_delete>
mock_requests_get = <function request at 0x7facc75b4670>
mock_get_access_credentials = <function get_access_credentials at 0x7facc75b4b80>

    @mock.patch('azure.cli.command_modules.acr.helm.get_access_credentials', autospec=True)
    @mock.patch('requests.request', autospec=True)
    def test_helm_delete(self, mock_requests_get, mock_get_access_credentials):
        cmd = self.setup_cmd()

        response = mock.MagicMock()
        response.headers = {}
        response.status_code = 200
        mock_requests_get.return_value = response

        mock_get_access_credentials.return_value = 'testregistry.azurecr.io', EMPTY_GUID, 'password'

        # Delete all versions of a chart
>       acr_helm_delete(cmd, 'testregistry', 'mychart1', repository='testrepository', yes=True)

src/azure-cli/azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:609:

src/azure-cli/azure/cli/command_modules/acr/helm.py:103: in acr_helm_delete
    return request_data_from_registry(
                                       _

http_method = 'delete', login_server = 'testregistry.azurecr.io'
path = '/helm/v1/testrepository/_charts/mychart1'
username = '00000000-0000-0000-0000-000000000000', password = 'password'
result_index = None, json_payload = None, file_payload = None, params = None
manifest_headers = False, raw = False, retry_times = 3, retry_interval = 5
timeout = 300

    def request_data_from_registry(http_method,
                                   login_server,
                                   path,
                                   username,
                                   password,
                                   result_index=None,
                                   json_payload=None,
                                   file_payload=None,
                                   params=None,
                                   manifest_headers=False,
                                   raw=False,
                                   retry_times=3,
                                   retry_interval=5,
                                   timeout=300):
        if http_method not in ALLOWED_HTTP_METHOD:
            raise ValueError("Allowed http method: {}".format(ALLOWED_HTTP_METHOD))

        if json_payload and file_payload:
            raise ValueError("One of json_payload and file_payload can be specified.")

        if http_method in ['get', 'delete'] and (json_payload or file_payload):
            raise ValueError("Empty payload is required for http method: {}".format(http_method))

        if http_method in ['patch', 'put'] and not (json_payload or file_payload):
            raise ValueError("Non-empty payload is required for http method: {}".format(http_method))

        url = 'https://{}{}'.format(login_server, path)

        if manifest_headers:
            headers = get_manifest_authorization_header(username, password)
        else:
            headers = get_authorization_header(username, password)

        for i in range(0, retry_times):
            errorMessage = None
            try:
                if file_payload:
                    with open(file_payload, 'rb') as data_payload:
                        logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                        response = session.request(
                            method=http_method,
                            url=url,
                            headers=headers,
                            params=params,
                            data=data_payload,
                            timeout=timeout,
                            verify=(not should_disable_connection_verify())
                        )
                else:
                    logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                    response = session.request(
                        method=http_method,
                        url=url,
                        headers=headers,
                        params=params,
                        json=json_payload,
                        timeout=timeout,
                        verify=(not should_disable_connection_verify())
                    )

                log_registry_response(response)

                if manifest_headers and raw and response.status_code == 200:
                    return response.content.decode('utf-8'), None, response.status_code
                if response.status_code == 200:
                    result = response.json()[result_index] if result_index else response.json()
                    next_link = response.headers['link'] if 'link' in response.headers else None
                    return result, next_link, response.status_code
                if response.status_code == 201 or response.status_code == 202:
                    result = None
                    try:
                        result = response.json()[result_index] if result_index else response.json()
                    except ValueError as e:
                        logger.debug('Response is empty or is not a valid json. Exception: %s', str(e))
                    return result, None, response.status_code
                if response.status_code == 204:
                    return None, None, response.status_code
                if response.status_code == 401:
>                   raise RegistryException(
                        parse_error_message('Authentication required.', response),
                        response.status_code)
E                       azure.cli.command_modules.acr._docker_utils.RegistryException: 2025-01-26 02:32:01.979752 Error: authentication required, visit https://aka.ms/acr/authorization for more information. Correlation ID: 714a3d91-ebe7-4fe1-932f-bddd81f3bf7d.

src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py:637: RegistryException azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:595

Failed test_helm_list self = <azure.cli.command_modules.acr.tests.hybrid_2020_09_01.test_acr_commands_mock.AcrMockCommandsTests testMethod=test_helm_list>
mock_requests_get = <function request at 0x7facc75b4670>
mock_get_access_credentials = <function get_access_credentials at 0x7facc75b4b80>

    @mock.patch('azure.cli.command_modules.acr.helm.get_access_credentials', autospec=True)
    @mock.patch('requests.request', autospec=True)
    def test_helm_list(self, mock_requests_get, mock_get_access_credentials):
        cmd = self.setup_cmd()

        response = mock.MagicMock()
        response.headers = {}
        response.status_code = 200
        response.content = json.dumps({
            'mychart1': [
                {
                    'name': 'mychart1',
                    'version': '0.2.1'
                },
                {
                    'name': 'mychart1',
                    'version': '0.1.2'
                }
            ],
            'mychart2': [
                {
                    'name': 'mychart2',
                    'version': '2.1.0'
                }
            ]}).encode()
        mock_requests_get.return_value = response

        mock_get_access_credentials.return_value = 'testregistry.azurecr.io', EMPTY_GUID, 'password'
>       acr_helm_list(cmd, 'testregistry', repository='testrepository')

src/azure-cli/azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:541:

src/azure-cli/azure/cli/command_modules/acr/helm.py:42: in acr_helm_list
    return request_data_from_registry(
                                       _

http_method = 'get', login_server = 'testregistry.azurecr.io'
path = '/helm/v1/testrepository/_charts'
username = '00000000-0000-0000-0000-000000000000', password = 'password'
result_index = None, json_payload = None, file_payload = None, params = None
manifest_headers = False, raw = False, retry_times = 3, retry_interval = 5
timeout = 300

    def request_data_from_registry(http_method,
                                   login_server,
                                   path,
                                   username,
                                   password,
                                   result_index=None,
                                   json_payload=None,
                                   file_payload=None,
                                   params=None,
                                   manifest_headers=False,
                                   raw=False,
                                   retry_times=3,
                                   retry_interval=5,
                                   timeout=300):
        if http_method not in ALLOWED_HTTP_METHOD:
            raise ValueError("Allowed http method: {}".format(ALLOWED_HTTP_METHOD))

        if json_payload and file_payload:
            raise ValueError("One of json_payload and file_payload can be specified.")

        if http_method in ['get', 'delete'] and (json_payload or file_payload):
            raise ValueError("Empty payload is required for http method: {}".format(http_method))

        if http_method in ['patch', 'put'] and not (json_payload or file_payload):
            raise ValueError("Non-empty payload is required for http method: {}".format(http_method))

        url = 'https://{}{}'.format(login_server, path)

        if manifest_headers:
            headers = get_manifest_authorization_header(username, password)
        else:
            headers = get_authorization_header(username, password)

        for i in range(0, retry_times):
            errorMessage = None
            try:
                if file_payload:
                    with open(file_payload, 'rb') as data_payload:
                        logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                        response = session.request(
                            method=http_method,
                            url=url,
                            headers=headers,
                            params=params,
                            data=data_payload,
                            timeout=timeout,
                            verify=(not should_disable_connection_verify())
                        )
                else:
                    logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                    response = session.request(
                        method=http_method,
                        url=url,
                        headers=headers,
                        params=params,
                        json=json_payload,
                        timeout=timeout,
                        verify=(not should_disable_connection_verify())
                    )

                log_registry_response(response)

                if manifest_headers and raw and response.status_code == 200:
                    return response.content.decode('utf-8'), None, response.status_code
                if response.status_code == 200:
                    result = response.json()[result_index] if result_index else response.json()
                    next_link = response.headers['link'] if 'link' in response.headers else None
                    return result, next_link, response.status_code
                if response.status_code == 201 or response.status_code == 202:
                    result = None
                    try:
                        result = response.json()[result_index] if result_index else response.json()
                    except ValueError as e:
                        logger.debug('Response is empty or is not a valid json. Exception: %s', str(e))
                    return result, None, response.status_code
                if response.status_code == 204:
                    return None, None, response.status_code
                if response.status_code == 401:
>                   raise RegistryException(
                        parse_error_message('Authentication required.', response),
                        response.status_code)
E                       azure.cli.command_modules.acr._docker_utils.RegistryException: 2025-01-26 02:32:02.482096 Error: authentication required, visit https://aka.ms/acr/authorization for more information. Correlation ID: 3a6bcae6-45f3-4784-91b7-702e72b167d6.

src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py:637: RegistryException azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:512

Failed test_helm_push self = <azure.cli.command_modules.acr.tests.hybrid_2020_09_01.test_acr_commands_mock.AcrMockCommandsTests testMethod=test_helm_push>
mock_requests_get = <function request at 0x7facc75b4670>
mock_get_access_credentials = <function get_access_credentials at 0x7facc75b4b80>

    @mock.patch('azure.cli.command_modules.acr.helm.get_access_credentials', autospec=True)
    @mock.patch('requests.request', autospec=True)
    def test_helm_push(self, mock_requests_get, mock_get_access_credentials):
        cmd = self.setup_cmd()

        response = mock.MagicMock()
        response.headers = {}
        response.status_code = 200
        mock_requests_get.return_value = response

        mock_get_access_credentials.return_value = 'testregistry.azurecr.io', EMPTY_GUID, 'password'

        builtins_open = 'builtin.open' if sys.version_info[0] < 3 else 'builtins.open'

        # Push a chart
        with mock.patch(builtins_open) as mock_open:
            mock_open.return_value = mock.MagicMock()
>           acr_helm_push(cmd, 'testregistry', './charts/mychart1-0.2.1.tgz', repository='testrepository')

src/azure-cli/azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:647:

src/azure-cli/azure/cli/command_modules/acr/helm.py:137: in acr_helm_push
    result = request_data_from_registry(
                                       _

http_method = 'put', login_server = 'testregistry.azurecr.io'
path = '/helm/v1/testrepository/_blobs/mychart1-0.2.1.tgz'
username = '00000000-0000-0000-0000-000000000000', password = 'password'
result_index = None, json_payload = None
file_payload = './charts/mychart1-0.2.1.tgz', params = None
manifest_headers = False, raw = False, retry_times = 3, retry_interval = 5
timeout = 300

    def request_data_from_registry(http_method,
                                   login_server,
                                   path,
                                   username,
                                   password,
                                   result_index=None,
                                   json_payload=None,
                                   file_payload=None,
                                   params=None,
                                   manifest_headers=False,
                                   raw=False,
                                   retry_times=3,
                                   retry_interval=5,
                                   timeout=300):
        if http_method not in ALLOWED_HTTP_METHOD:
            raise ValueError("Allowed http method: {}".format(ALLOWED_HTTP_METHOD))

        if json_payload and file_payload:
            raise ValueError("One of json_payload and file_payload can be specified.")

        if http_method in ['get', 'delete'] and (json_payload or file_payload):
            raise ValueError("Empty payload is required for http method: {}".format(http_method))

        if http_method in ['patch', 'put'] and not (json_payload or file_payload):
            raise ValueError("Non-empty payload is required for http method: {}".format(http_method))

        url = 'https://{}{}'.format(login_server, path)

        if manifest_headers:
            headers = get_manifest_authorization_header(username, password)
        else:
            headers = get_authorization_header(username, password)

        for i in range(0, retry_times):
            errorMessage = None
            try:
                if file_payload:
                    with open(file_payload, 'rb') as data_payload:
                        logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                        response = session.request(
                            method=http_method,
                            url=url,
                            headers=headers,
                            params=params,
                            data=data_payload,
                            timeout=timeout,
                            verify=(not should_disable_connection_verify())
                        )
                else:
                    logger.debug(add_timestamp("Sending a HTTP {} request to {}".format(http_method, url)))
                    response = session.request(
                        method=http_method,
                        url=url,
                        headers=headers,
                        params=params,
                        json=json_payload,
                        timeout=timeout,
                        verify=(not should_disable_connection_verify())
                    )

                log_registry_response(response)

                if manifest_headers and raw and response.status_code == 200:
                    return response.content.decode('utf-8'), None, response.status_code
                if response.status_code == 200:
                    result = response.json()[result_index] if result_index else response.json()
                    next_link = response.headers['link'] if 'link' in response.headers else None
                    return result, next_link, response.status_code
                if response.status_code == 201 or response.status_code == 202:
                    result = None
                    try:
                        result = response.json()[result_index] if result_index else response.json()
                    except ValueError as e:
                        logger.debug('Response is empty or is not a valid json. Exception: %s', str(e))
                    return result, None, response.status_code
                if response.status_code == 204:
                    return None, None, response.status_code
                if response.status_code == 401:
>                   raise RegistryException(
                        parse_error_message('Authentication required.', response),
                        response.status_code)
E                       azure.cli.command_modules.acr._docker_utils.RegistryException: 2025-01-26 02:32:02.914315 Error: authentication required, visit https://aka.ms/acr/authorization for more information. Correlation ID: e805162c-d67c-4f24-ba5c-bb47f879f9f9.

src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py:637: RegistryException azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:629

Failed test_helm_show The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:550

Failed test_repository_delete The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:317

Failed test_repository_list The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:50

Failed test_repository_show The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:253

Failed test_repository_show_manifests The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:147

Failed test_repository_show_tags The error message is too long, please check the pipeline log for details. azure/cli/command_modules/acr/tests/hybrid_2020_09_01/test_acr_commands_mock.py:91

️✔️latest

️✔️3.12

️✔️3.9

️✔️acs

️✔️2020-09-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️advisor

️✔️latest

️✔️3.12

️✔️3.9

️✔️ams

️✔️latest

️✔️3.12

️✔️3.9

️✔️apim

️✔️latest

️✔️3.12

️✔️3.9

️✔️appconfig

️✔️latest

️✔️3.12

️✔️3.9

️✔️appservice

️✔️latest

️✔️3.12

️✔️3.9

️✔️aro

️✔️latest

️✔️3.12

️✔️3.9

️✔️backup

️✔️latest

️✔️3.12

️✔️3.9

️✔️batch

️✔️latest

️✔️3.12

️✔️3.9

️✔️batchai

️✔️latest

️✔️3.12

️✔️3.9

️✔️billing

️✔️latest

️✔️3.12

️✔️3.9

️✔️botservice

️✔️latest

️✔️3.12

️✔️3.9

️✔️cdn

️✔️latest

️✔️3.12

️✔️3.9

️✔️cloud

️✔️latest

️✔️3.12

️✔️3.9

️✔️cognitiveservices

️✔️latest

️✔️3.12

️✔️3.9

️✔️compute_recommender

️✔️latest

️✔️3.12

️✔️3.9

️✔️computefleet

️✔️latest

️✔️3.12

️✔️3.9

️✔️config

️✔️latest

️✔️3.12

️✔️3.9

️✔️configure

️✔️latest

️✔️3.12

️✔️3.9

️✔️consumption

️✔️latest

️✔️3.12

️✔️3.9

️✔️container

️✔️latest

️✔️3.12

️✔️3.9

️✔️containerapp

️✔️latest

️✔️3.12

️✔️3.9

️✔️core

️✔️2018-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2019-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2020-09-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️cosmosdb

️✔️latest

️✔️3.12

️✔️3.9

️✔️databoxedge

️✔️2019-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2020-09-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️dls

️✔️latest

️✔️3.12

️✔️3.9

️✔️dms

️✔️latest

️✔️3.12

️✔️3.9

️✔️eventgrid

️✔️latest

️✔️3.12

️✔️3.9

️✔️eventhubs

️✔️latest

️✔️3.12

️✔️3.9

️✔️feedback

️✔️latest

️✔️3.12

️✔️3.9

️✔️find

️✔️latest

️✔️3.12

️✔️3.9

️✔️hdinsight

️✔️latest

️✔️3.12

️✔️3.9

️✔️identity

️✔️latest

️✔️3.12

️✔️3.9

️✔️iot

️✔️2019-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2020-09-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️keyvault

️✔️2018-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2020-09-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️lab

️✔️latest

️✔️3.12

️✔️3.9

️✔️managedservices

️✔️latest

️✔️3.12

️✔️3.9

️✔️maps

️✔️latest

️✔️3.12

️✔️3.9

️✔️marketplaceordering

️✔️latest

️✔️3.12

️✔️3.9

️✔️monitor

️✔️latest

️✔️3.12

️✔️3.9

️✔️mysql

️✔️latest

️✔️3.12

️✔️3.9

️✔️netappfiles

️✔️latest

️✔️3.12

️✔️3.9

️✔️network

️✔️2018-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️policyinsights

️✔️latest

️✔️3.12

️✔️3.9

️✔️privatedns

️✔️latest

️✔️3.12

️✔️3.9

️✔️profile

️✔️latest

️✔️3.12

️✔️3.9

️✔️rdbms

️✔️latest

️✔️3.12

️✔️3.9

️✔️redis

️✔️latest

️✔️3.12

️✔️3.9

️✔️relay

️✔️latest

️✔️3.12

️✔️3.9

️✔️resource

️✔️2018-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2019-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️role

️✔️latest

️✔️3.12

️✔️3.9

️✔️search

️✔️latest

️✔️3.12

️✔️3.9

️✔️security

️✔️latest

️✔️3.12

️✔️3.9

️✔️servicebus

️✔️latest

️✔️3.12

️✔️3.9

️✔️serviceconnector

️✔️latest

️✔️3.12

️✔️3.9

️✔️servicefabric

️✔️latest

️✔️3.12

️✔️3.9

️✔️signalr

️✔️latest

️✔️3.12

️✔️3.9

️✔️sql

️✔️latest

️✔️3.12

️✔️3.9

️✔️sqlvm

️✔️latest

️✔️3.12

️✔️3.9

️✔️storage

️✔️2018-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2019-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2020-09-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️synapse

️✔️latest

️✔️3.12

️✔️3.9

️✔️telemetry

️✔️2018-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2019-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2020-09-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

️✔️util

️✔️latest

️✔️3.12

️✔️3.9

️✔️vm

️✔️2018-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2019-03-01-hybrid

️✔️3.12

️✔️3.9

️✔️2020-09-01-hybrid

️✔️3.12

️✔️3.9

️✔️latest

️✔️3.12

️✔️3.9

azure-client-tools-bot-prd · 2024-12-15T19:33:26Z

️✔️AzureCLI-BreakingChangeTest

️✔️Non Breaking Changes

yonzhan · 2024-12-15T19:33:35Z

Thank you for your contribution! We will review the pull request and get back to you soon.

github-actions · 2024-12-15T19:33:35Z

⚠️Your changes in this PR will be released on Jan 14, 2025 due to CCOA (extend to Jan 6, 2025)

yanzhudd · 2025-01-24T08:17:50Z

/azp run

azure-pipelines · 2025-01-24T08:18:03Z

Azure Pipelines successfully started running 3 pipeline(s).

yanzhudd · 2025-01-24T08:18:27Z

Please rerun some related tests in live mode to verify this change.

yanzhudd · 2025-01-24T08:19:25Z

Please note Azure CLI will freeze the code on 01/28/2025 10:00 UTC for the upcoming release. If you want to catch this release train, please get this PR ready ASAP, otherwise it has to be postponed to next sprint.

oxpa · 2025-01-24T13:11:15Z

@microsoft-github-policy-service agree

oxpa · 2025-01-24T13:15:49Z

@yanzhudd i didn't realize there are tests available to me. I made sure to run them locally and it should all be good now.

yanzhudd · 2025-01-26T02:21:35Z

/azp run

azure-pipelines · 2025-01-26T02:21:49Z

Azure Pipelines successfully started running 3 pipeline(s).

yanzhudd · 2025-01-26T02:46:57Z

please fix the CI issues

oxpa · 2025-01-26T13:24:44Z

@yanzhudd pipelines run tests without my patches and I can't fix them otherwise, I think.

Tests mock requests.request object. While the code now uses azure.cli.command_modules.acr._docker_utils.session.request object. I changed tests to match the code in my last commit but it was not applied to tests in pipelines.

toddysm · 2025-01-28T18:10:24Z

@johnsonshi This is performance related. Can you please take a look at it?

yanzhudd · 2025-02-24T08:08:54Z

/azp run

johnsonshi · 2025-02-24T20:07:47Z

@m5i-work, can you take a look at this PR as it relates to a perf improvement using connection pooling when connecting to ACR using cli?

m5i-work · 2025-02-25T08:25:56Z

src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py

@@ -34,6 +35,7 @@


 logger = get_logger(__name__)
+session = Session()


Instead of a global var, can we move this to parameters of request_data_from_registry()? The new parameter can default to None. We can make this PR scoped to repository listing and revisit other call sites of request_data_from_registry() that can benefit from connection pooling later.

m5i-work · 2025-02-25T08:26:37Z

@oxpa Thanks for looking into this!

oxpa requested review from zhoxing-ms, jsntcy and yanzhudd as code owners December 15, 2024 19:33

azure-client-tools-bot-prd bot added this to the January 2025 (2025-01-14) milestone Dec 15, 2024

microsoft-github-policy-service bot requested review from wangzelin007 and yonzhan December 15, 2024 19:33

microsoft-github-policy-service bot added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Dec 15, 2024

microsoft-github-policy-service bot assigned zhoxing-ms Dec 15, 2024

microsoft-github-policy-service bot added Auto-Assign Auto assign by bot Container Registry az acr labels Dec 15, 2024

zhoxing-ms modified the milestones: January 2025 (2025-01-14), February 2025 (2025-02-11) Jan 8, 2025

oxpa added 3 commits January 24, 2025 11:54

{ACR} Add connection pooling with ACR registries.

f13d0ae

Use session imported from requests.

7736167

Update tests to work with new Session object in acr.

c7e9c36

oxpa force-pushed the dev branch from a347973 to c7e9c36 Compare January 24, 2025 13:09

yanzhudd modified the milestones: February 2025 (2025-02-11), March 2025 (2025-03-04) Jan 27, 2025

toddysm assigned johnsonshi Jan 28, 2025

zhoxing-ms modified the milestones: March 2025 (2025-03-04), April 2025 (2025-04-01) Feb 25, 2025

m5i-work reviewed Feb 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

{ACR} Add connection pooling with ACR registries. #30520

{ACR} Add connection pooling with ACR registries. #30520

oxpa commented Dec 15, 2024 •

edited

Loading

azure-client-tools-bot-prd bot commented Dec 15, 2024 •

edited

Loading

azure-client-tools-bot-prd bot commented Dec 15, 2024 •

edited

Loading

yonzhan commented Dec 15, 2024

github-actions bot commented Dec 15, 2024

yanzhudd commented Jan 24, 2025

azure-pipelines bot commented Jan 24, 2025

yanzhudd commented Jan 24, 2025

yanzhudd commented Jan 24, 2025

oxpa commented Jan 24, 2025

oxpa commented Jan 24, 2025

yanzhudd commented Jan 26, 2025

azure-pipelines bot commented Jan 26, 2025

yanzhudd commented Jan 26, 2025

oxpa commented Jan 26, 2025

toddysm commented Jan 28, 2025

yanzhudd commented Feb 24, 2025

johnsonshi commented Feb 24, 2025

m5i-work Feb 25, 2025

m5i-work commented Feb 25, 2025

		@@ -34,6 +35,7 @@


		logger = get_logger(__name__)
		session = Session()

{ACR} Add connection pooling with ACR registries. #30520

Are you sure you want to change the base?

{ACR} Add connection pooling with ACR registries. #30520

Conversation

oxpa commented Dec 15, 2024 • edited Loading

azure-client-tools-bot-prd bot commented Dec 15, 2024 • edited Loading

azure-client-tools-bot-prd bot commented Dec 15, 2024 • edited Loading

yonzhan commented Dec 15, 2024

github-actions bot commented Dec 15, 2024

yanzhudd commented Jan 24, 2025

azure-pipelines bot commented Jan 24, 2025

yanzhudd commented Jan 24, 2025

yanzhudd commented Jan 24, 2025

oxpa commented Jan 24, 2025

oxpa commented Jan 24, 2025

yanzhudd commented Jan 26, 2025

azure-pipelines bot commented Jan 26, 2025

yanzhudd commented Jan 26, 2025

oxpa commented Jan 26, 2025

toddysm commented Jan 28, 2025

yanzhudd commented Feb 24, 2025

johnsonshi commented Feb 24, 2025

m5i-work Feb 25, 2025

Choose a reason for hiding this comment

m5i-work commented Feb 25, 2025

oxpa commented Dec 15, 2024 •

edited

Loading

azure-client-tools-bot-prd bot commented Dec 15, 2024 •

edited

Loading

azure-client-tools-bot-prd bot commented Dec 15, 2024 •

edited

Loading