From 973a69613cb3e010dfaa926ef1902165c927cc8b Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 21 Jan 2022 15:01:31 -0500 Subject: [PATCH 1/9] add operation location final get --- .../azure/core/polling/base_polling.py | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core/azure/core/polling/base_polling.py b/sdk/core/azure-core/azure/core/polling/base_polling.py index 6b6fd4ff7c93..69d6a73f5791 100644 --- a/sdk/core/azure-core/azure/core/polling/base_polling.py +++ b/sdk/core/azure-core/azure/core/polling/base_polling.py @@ -26,6 +26,7 @@ import abc import base64 import json +from enum import Enum from typing import TYPE_CHECKING, Optional, Any, Union from ..exceptions import HttpResponseError, DecodeError @@ -174,6 +175,19 @@ def get_final_get_url(self, pipeline_response): """ raise NotImplementedError() +class _LroOption(str, Enum): + """Known LRO options from Swagger.""" + + FINAL_STATE_VIA = "final-state-via" + + +class _FinalStateViaOption(str, Enum): + """Possible final-state-via options.""" + + AZURE_ASYNC_OPERATION_FINAL_STATE = "azure-async-operation" + LOCATION_FINAL_STATE = "location" + OPERATION_LOCATION_FINAL_STATE = "operation-location" + class OperationResourcePolling(LongRunningOperation): """Implements a operation resource polling, typically from Operation-Location. @@ -181,13 +195,16 @@ class OperationResourcePolling(LongRunningOperation): :param str operation_location_header: Name of the header to return operation format (default 'operation-location') """ - def __init__(self, operation_location_header="operation-location"): + def __init__( + self, operation_location_header="operation-location", **kwargs + ): self._operation_location_header = operation_location_header # Store the initial URLs self._async_url = None self._location_url = None self._request = None + self._lro_options = kwargs.pop("lro_options", {}) or {} def can_poll(self, pipeline_response): """Answer if this polling method could be used. @@ -207,6 +224,15 @@ def get_final_get_url(self, pipeline_response): :rtype: str """ + if ( + self._lro_options.get(_LroOption.FINAL_STATE_VIA) + in [ + _FinalStateViaOption.AZURE_ASYNC_OPERATION_FINAL_STATE, + _FinalStateViaOption.OPERATION_LOCATION_FINAL_STATE + ] + and self._request.method == "POST" + ): + return None response = pipeline_response.http_response if not _is_empty(response): body = _as_json(response) @@ -381,7 +407,7 @@ def __init__( **operation_config ): self._lro_algorithms = lro_algorithms or [ - OperationResourcePolling(), + OperationResourcePolling(lro_options=lro_options), LocationPolling(), StatusCheckPolling(), ] From 57de03a1d4517c7b08cd76b23cfa075c49ce05c7 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 21 Jan 2022 15:19:32 -0500 Subject: [PATCH 2/9] add operation location polling test --- .../async_tests/test_base_polling_async.py | 103 ++++++++++++++++++ .../azure-core/tests/test_base_polling.py | 103 ++++++++++++++++++ 2 files changed, 206 insertions(+) diff --git a/sdk/core/azure-core/tests/async_tests/test_base_polling_async.py b/sdk/core/azure-core/tests/async_tests/test_base_polling_async.py index 82a5d21bb4d4..7d858768040b 100644 --- a/sdk/core/azure-core/tests/async_tests/test_base_polling_async.py +++ b/sdk/core/azure-core/tests/async_tests/test_base_polling_async.py @@ -748,3 +748,106 @@ async def test_long_running_negative(http_request, http_response): LOCATION_BODY = json.dumps({ 'name': TEST_NAME }) POLLING_STATUS = 200 +@pytest.mark.asyncio +@pytest.mark.parametrize("http_request,http_response", request_and_responses_product(ASYNCIO_REQUESTS_TRANSPORT_RESPONSES)) +async def test_post_final_state_via(async_pipeline_client_builder, deserialization_cb, http_request, http_response): + # Test POST LRO with both Location and Operation-Location + CLIENT.http_request_type = http_request + CLIENT.http_response_type = http_response + # The initial response contains both Location and Operation-Location, a 202 and no Body + initial_response = TestBasePolling.mock_send( + http_request, + http_response, + 'POST', + 202, + { + 'location': 'http://example.org/location', + 'operation-location': 'http://example.org/async_monitor', + }, + '' + ) + + async def send(request, **kwargs): + assert request.method == 'GET' + + if request.url == 'http://example.org/location': + return TestBasePolling.mock_send( + http_request, + http_response, + 'GET', + 200, + body={'location_result': True} + ).http_response + elif request.url == 'http://example.org/async_monitor': + return TestBasePolling.mock_send( + http_request, + http_response, + 'GET', + 200, + body={'status': 'Succeeded'} + ).http_response + else: + pytest.fail("No other query allowed") + + client = async_pipeline_client_builder(send) + + # Test 1, LRO options with Location final state + poll = async_poller( + client, + initial_response, + deserialization_cb, + AsyncLROBasePolling(0, lro_options={"final-state-via": "location"})) + result = await poll + assert result['location_result'] == True + + # Test 2, LRO options with Operation-Location final state + poll = async_poller( + client, + initial_response, + deserialization_cb, + AsyncLROBasePolling(0, lro_options={"final-state-via": "operation-location"})) + result = await poll + assert result['status'] == 'Succeeded' + + # Test 3, "do the right thing" and use Location by default + poll = async_poller( + client, + initial_response, + deserialization_cb, + AsyncLROBasePolling(0)) + result = await poll + assert result['location_result'] == True + + # Test 4, location has no body + + async def send(request, **kwargs): + assert request.method == 'GET' + + if request.url == 'http://example.org/location': + return TestBasePolling.mock_send( + http_request, + http_response, + 'GET', + 200, + body=None + ).http_response + elif request.url == 'http://example.org/async_monitor': + return TestBasePolling.mock_send( + http_request, + http_response, + 'GET', + 200, + body={'status': 'Succeeded'} + ).http_response + else: + pytest.fail("No other query allowed") + + client = async_pipeline_client_builder(send) + + poll = async_poller( + client, + initial_response, + deserialization_cb, + AsyncLROBasePolling(0, lro_options={"final-state-via": "location"})) + result = await poll + assert result is None diff --git a/sdk/core/azure-core/tests/test_base_polling.py b/sdk/core/azure-core/tests/test_base_polling.py index ad030099c6eb..e8217164c7e6 100644 --- a/sdk/core/azure-core/tests/test_base_polling.py +++ b/sdk/core/azure-core/tests/test_base_polling.py @@ -756,3 +756,106 @@ def test_long_running_negative(self, http_request, http_response): LOCATION_BODY = json.dumps({ 'name': TEST_NAME }) POLLING_STATUS = 200 + + @pytest.mark.parametrize("http_request,http_response", request_and_responses_product(REQUESTS_TRANSPORT_RESPONSES)) + def test_post_final_state_via(self, pipeline_client_builder, deserialization_cb, http_request, http_response): + # Test POST LRO with both Location and Operation-Location + CLIENT.http_request_type = http_request + CLIENT.http_response_type = http_response + # The initial response contains both Location and Operation-Location, a 202 and no Body + initial_response = TestBasePolling.mock_send( + http_request, + http_response, + 'POST', + 202, + { + 'location': 'http://example.org/location', + 'operation-location': 'http://example.org/async_monitor', + }, + '' + ) + + def send(request, **kwargs): + assert request.method == 'GET' + + if request.url == 'http://example.org/location': + return TestBasePolling.mock_send( + http_request, + http_response, + 'GET', + 200, + body={'location_result': True} + ).http_response + elif request.url == 'http://example.org/async_monitor': + return TestBasePolling.mock_send( + http_request, + http_response, + 'GET', + 200, + body={'status': 'Succeeded'} + ).http_response + else: + pytest.fail("No other query allowed") + + client = pipeline_client_builder(send) + + # Test 1, LRO options with Location final state + poll = LROPoller( + client, + initial_response, + deserialization_cb, + LROBasePolling(0, lro_options={"final-state-via": "location"})) + result = poll.result() + assert result['location_result'] == True + + # Test 2, LRO options with Operation-Location final state + poll = LROPoller( + client, + initial_response, + deserialization_cb, + LROBasePolling(0, lro_options={"final-state-via": "operation-location"})) + result = poll.result() + assert result['status'] == 'Succeeded' + + # Test 3, "do the right thing" and use Location by default + poll = LROPoller( + client, + initial_response, + deserialization_cb, + LROBasePolling(0)) + result = poll.result() + assert result['location_result'] == True + + # Test 4, location has no body + + def send(request, **kwargs): + assert request.method == 'GET' + + if request.url == 'http://example.org/location': + return TestBasePolling.mock_send( + http_request, + http_response, + 'GET', + 200, + body=None + ).http_response + elif request.url == 'http://example.org/async_monitor': + return TestBasePolling.mock_send( + http_request, + http_response, + 'GET', + 200, + body={'status': 'Succeeded'} + ).http_response + else: + pytest.fail("No other query allowed") + + client = pipeline_client_builder(send) + + poll = LROPoller( + client, + initial_response, + deserialization_cb, + LROBasePolling(0, lro_options={"final-state-via": "location"})) + result = poll.result() + assert result is None From 45c5030dd98bf5ab0c025e1ea673339606cbe987 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 21 Jan 2022 15:41:42 -0500 Subject: [PATCH 3/9] update changelog --- sdk/core/azure-core/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index c6d82e9509f5..fb28a1917d86 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features Added +- Add support for `final-state-via` LRO option in core. #22713 + ### Breaking Changes ### Bugs Fixed From ff296d639a8293bc5862693aa99b689313009fc7 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 28 Jan 2022 12:53:30 -0500 Subject: [PATCH 4/9] add test for location url --- .../async_tests/test_base_polling_async.py | 22 +++++++++++++++++- .../azure-core/tests/test_base_polling.py | 23 ++++++++++++++++++- .../coretestserver/test_routes/polling.py | 19 +++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core/tests/async_tests/test_base_polling_async.py b/sdk/core/azure-core/tests/async_tests/test_base_polling_async.py index 7d858768040b..ed16f4712701 100644 --- a/sdk/core/azure-core/tests/async_tests/test_base_polling_async.py +++ b/sdk/core/azure-core/tests/async_tests/test_base_polling_async.py @@ -42,7 +42,7 @@ from msrest import Deserializer -from azure.core.polling import async_poller +from azure.core.polling import async_poller, AsyncLROPoller from azure.core.exceptions import DecodeError, HttpResponseError from azure.core import AsyncPipelineClient from azure.core.pipeline import PipelineResponse, AsyncPipeline, PipelineContext @@ -52,6 +52,7 @@ AsyncLROBasePolling, ) from utils import ASYNCIO_REQUESTS_TRANSPORT_RESPONSES, request_and_responses_product, create_transport_response +from rest_client_async import AsyncTestRestClient class SimpleResource: """An implementation of Python 3 SimpleNamespace. @@ -851,3 +852,22 @@ async def send(request, **kwargs): AsyncLROBasePolling(0, lro_options={"final-state-via": "location"})) result = await poll assert result is None + +@pytest.mark.asyncio +@pytest.mark.parametrize("http_request", HTTP_REQUESTS) +async def test_final_get_via_location(port, http_request, deserialization_cb): + client = AsyncTestRestClient(port) + request = http_request( + "PUT", + "http://localhost:{}/polling/polling-with-options".format(port), + ) + request.set_json_body({"hello": "world!"}) + initial_response = await client._client._pipeline.run(request) + poller = AsyncLROPoller( + client._client, + initial_response, + deserialization_cb, + AsyncLROBasePolling(0, lro_options={"final-state-via": "location"}), + ) + result = await poller.result() + assert result == {"returnedFrom": "locationHeaderUrl"} diff --git a/sdk/core/azure-core/tests/test_base_polling.py b/sdk/core/azure-core/tests/test_base_polling.py index e8217164c7e6..0131bea62391 100644 --- a/sdk/core/azure-core/tests/test_base_polling.py +++ b/sdk/core/azure-core/tests/test_base_polling.py @@ -31,6 +31,9 @@ import pickle import platform import six +from tests.rest_client import TestRestClient + +from tests.utils import HTTP_REQUESTS try: from unittest import mock except ImportError: @@ -44,7 +47,7 @@ from azure.core.exceptions import DecodeError, HttpResponseError from azure.core import PipelineClient from azure.core.pipeline import PipelineResponse, Pipeline, PipelineContext -from azure.core.pipeline.transport import HttpTransport +from azure.core.pipeline.transport import HttpTransport, RequestsTransport from azure.core.polling.base_polling import LROBasePolling from azure.core.pipeline.policies._utils import _FixedOffset @@ -859,3 +862,21 @@ def send(request, **kwargs): LROBasePolling(0, lro_options={"final-state-via": "location"})) result = poll.result() assert result is None + +@pytest.mark.parametrize("http_request", HTTP_REQUESTS) +def test_final_get_via_location(port, http_request, deserialization_cb): + client = TestRestClient(port) + request = http_request( + "PUT", + "http://localhost:{}/polling/polling-with-options".format(port), + ) + request.set_json_body({"hello": "world!"}) + initial_response = client._client._pipeline.run(request) + poller = LROPoller( + client._client, + initial_response, + deserialization_cb, + LROBasePolling(0, lro_options={"final-state-via": "location"}), + ) + result = poller.result() + assert result == {"returnedFrom": "locationHeaderUrl"} diff --git a/sdk/core/azure-core/tests/testserver_tests/coretestserver/coretestserver/test_routes/polling.py b/sdk/core/azure-core/tests/testserver_tests/coretestserver/coretestserver/test_routes/polling.py index c433bfb4d04d..326f2dce63dc 100644 --- a/sdk/core/azure-core/tests/testserver_tests/coretestserver/coretestserver/test_routes/polling.py +++ b/sdk/core/azure-core/tests/testserver_tests/coretestserver/coretestserver/test_routes/polling.py @@ -151,3 +151,22 @@ def request_id_location(): '{"status": "Succeeded"}', status=200 ) + +@polling_api.route('/polling-with-options', methods=["PUT"]) +def polling_with_options_first(): + base_url = get_base_url(request) + return Response( + '{"properties":{"provisioningState": "InProgress"}}', + headers={ + 'location': '{}/polling/final-get-with-location'.format(base_url), + 'operation-location': '{}/polling/post/resource-location/operation-location-url'.format(base_url), + }, + status=202 + ) + +@polling_api.route('/final-get-with-location', methods=["GET"]) +def polling_with_options_final_get_with_location(): + return Response( + '{"returnedFrom": "locationHeaderUrl"}', + status=200 + ) From aac916263d31d9c9a9b293760cc5ac6b31c4f029 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 28 Jan 2022 13:03:12 -0500 Subject: [PATCH 5/9] return location url if present --- sdk/core/azure-core/azure/core/polling/base_polling.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sdk/core/azure-core/azure/core/polling/base_polling.py b/sdk/core/azure-core/azure/core/polling/base_polling.py index 69d6a73f5791..22728e74f0bd 100644 --- a/sdk/core/azure-core/azure/core/polling/base_polling.py +++ b/sdk/core/azure-core/azure/core/polling/base_polling.py @@ -224,13 +224,17 @@ def get_final_get_url(self, pipeline_response): :rtype: str """ + if ( + self._lro_options.get(_LroOption.FINAL_STATE_VIA) == _FinalStateViaOption.LOCATION_FINAL_STATE + and self._location_url + ): + return self._location_url if ( self._lro_options.get(_LroOption.FINAL_STATE_VIA) in [ _FinalStateViaOption.AZURE_ASYNC_OPERATION_FINAL_STATE, _FinalStateViaOption.OPERATION_LOCATION_FINAL_STATE ] - and self._request.method == "POST" ): return None response = pipeline_response.http_response From e21103c2b03cd95a8c905fb033dfcfbb04cb400b Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 28 Jan 2022 13:35:20 -0500 Subject: [PATCH 6/9] add docstrings for lro_options --- sdk/core/azure-core/azure/core/polling/base_polling.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/core/azure-core/azure/core/polling/base_polling.py b/sdk/core/azure-core/azure/core/polling/base_polling.py index 22728e74f0bd..494de7c954c3 100644 --- a/sdk/core/azure-core/azure/core/polling/base_polling.py +++ b/sdk/core/azure-core/azure/core/polling/base_polling.py @@ -193,6 +193,7 @@ class OperationResourcePolling(LongRunningOperation): """Implements a operation resource polling, typically from Operation-Location. :param str operation_location_header: Name of the header to return operation format (default 'operation-location') + :keyword dict[str, any] lro_options: Additional options for LRO """ def __init__( From 7c2e9ce617c9afe31c203b48fe5fce6fc1325d2c Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 28 Jan 2022 13:36:14 -0500 Subject: [PATCH 7/9] fix imports in test --- sdk/core/azure-core/tests/test_base_polling.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure-core/tests/test_base_polling.py b/sdk/core/azure-core/tests/test_base_polling.py index 0131bea62391..7d5fcea28ab3 100644 --- a/sdk/core/azure-core/tests/test_base_polling.py +++ b/sdk/core/azure-core/tests/test_base_polling.py @@ -31,9 +31,7 @@ import pickle import platform import six -from tests.rest_client import TestRestClient -from tests.utils import HTTP_REQUESTS try: from unittest import mock except ImportError: @@ -47,11 +45,11 @@ from azure.core.exceptions import DecodeError, HttpResponseError from azure.core import PipelineClient from azure.core.pipeline import PipelineResponse, Pipeline, PipelineContext -from azure.core.pipeline.transport import HttpTransport, RequestsTransport +from azure.core.pipeline.transport import HttpTransport from azure.core.polling.base_polling import LROBasePolling from azure.core.pipeline.policies._utils import _FixedOffset -from utils import request_and_responses_product, REQUESTS_TRANSPORT_RESPONSES, create_transport_response +from utils import request_and_responses_product, REQUESTS_TRANSPORT_RESPONSES, create_transport_response, HTTP_REQUESTS from azure.core.pipeline._tools import is_rest class SimpleResource: From c0259f2061e0d9225d5b1bf6cf1f1aab24627f33 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 28 Jan 2022 13:47:41 -0500 Subject: [PATCH 8/9] add aka link for lro options --- sdk/core/azure-core/azure/core/polling/base_polling.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/core/azure-core/azure/core/polling/base_polling.py b/sdk/core/azure-core/azure/core/polling/base_polling.py index 494de7c954c3..01b8e4789768 100644 --- a/sdk/core/azure-core/azure/core/polling/base_polling.py +++ b/sdk/core/azure-core/azure/core/polling/base_polling.py @@ -193,7 +193,8 @@ class OperationResourcePolling(LongRunningOperation): """Implements a operation resource polling, typically from Operation-Location. :param str operation_location_header: Name of the header to return operation format (default 'operation-location') - :keyword dict[str, any] lro_options: Additional options for LRO + :keyword dict[str, any] lro_options: Additional options for LRO. For more information, see + https://aka.ms/azsdk/autorest/openapi/lro-options """ def __init__( From 32a5819fea6b3aae131e948524d471f75b6bf7e4 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Fri, 28 Jan 2022 14:34:43 -0500 Subject: [PATCH 9/9] add back TestRestClient import --- sdk/core/azure-core/tests/test_base_polling.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/core/azure-core/tests/test_base_polling.py b/sdk/core/azure-core/tests/test_base_polling.py index 7d5fcea28ab3..47f947ef63cb 100644 --- a/sdk/core/azure-core/tests/test_base_polling.py +++ b/sdk/core/azure-core/tests/test_base_polling.py @@ -51,6 +51,7 @@ from azure.core.pipeline.policies._utils import _FixedOffset from utils import request_and_responses_product, REQUESTS_TRANSPORT_RESPONSES, create_transport_response, HTTP_REQUESTS from azure.core.pipeline._tools import is_rest +from rest_client import TestRestClient class SimpleResource: """An implementation of Python 3 SimpleNamespace.