From 6f03aabe55eeee03cd51984403e43950be8b8012 Mon Sep 17 00:00:00 2001 From: Pavan-Microsoft Date: Sat, 28 Sep 2024 00:52:46 +0530 Subject: [PATCH] fix: Fix for byod without data issue (#1355) Co-authored-by: Pavan Kumar --- .../batch/utilities/helpers/env_helper.py | 9 ---- code/create_app.py | 17 ++++++- .../with_byod/test_conversation_flow.py | 12 ++++- code/tests/test_app.py | 48 +++++++++++++++---- 4 files changed, 66 insertions(+), 20 deletions(-) diff --git a/code/backend/batch/utilities/helpers/env_helper.py b/code/backend/batch/utilities/helpers/env_helper.py index 99b742e61..b63f606c7 100644 --- a/code/backend/batch/utilities/helpers/env_helper.py +++ b/code/backend/batch/utilities/helpers/env_helper.py @@ -254,15 +254,6 @@ def __load_config(self, **kwargs) -> None: self.PROMPT_FLOW_DEPLOYMENT_NAME = os.getenv("PROMPT_FLOW_DEPLOYMENT_NAME", "") - def should_use_data(self) -> bool: - if ( - self.AZURE_SEARCH_SERVICE - and self.AZURE_SEARCH_INDEX - and (self.AZURE_SEARCH_KEY or self.AZURE_AUTH_TYPE == "rbac") - ): - return True - return False - def is_chat_model(self): if "gpt-4" in self.AZURE_OPENAI_MODEL_NAME.lower(): return True diff --git a/code/create_app.py b/code/create_app.py index 896db501c..daed4eb6c 100644 --- a/code/create_app.py +++ b/code/create_app.py @@ -16,6 +16,7 @@ from dotenv import load_dotenv from urllib.parse import quote from backend.batch.utilities.helpers.env_helper import EnvHelper +from backend.batch.utilities.helpers.azure_search_helper import AzureSearchHelper from backend.batch.utilities.helpers.orchestrator_helper import Orchestrator from backend.batch.utilities.helpers.config.config_helper import ConfigHelper from backend.batch.utilities.helpers.config.conversation_flow import ConversationFlow @@ -69,6 +70,19 @@ def get_citations(citation_list): return citations_dict +def should_use_data( + env_helper: EnvHelper, azure_search_helper: AzureSearchHelper +) -> bool: + if ( + env_helper.AZURE_SEARCH_SERVICE + and env_helper.AZURE_SEARCH_INDEX + and (env_helper.AZURE_SEARCH_KEY or env_helper.AZURE_AUTH_TYPE == "rbac") + and not azure_search_helper._index_not_exists(env_helper.AZURE_SEARCH_INDEX) + ): + return True + return False + + def stream_with_data(response: Stream[ChatCompletionChunk]): """This function streams the response from Azure OpenAI with data.""" response_obj = { @@ -371,6 +385,7 @@ def create_app(): app = Flask(__name__) env_helper: EnvHelper = EnvHelper() + azure_search_helper: AzureSearchHelper = AzureSearchHelper() logger.debug("Starting web app") @@ -385,7 +400,7 @@ def health(): def conversation_azure_byod(): try: - if env_helper.should_use_data(): + if should_use_data(env_helper, azure_search_helper): return conversation_with_data(request, env_helper) else: return conversation_without_data(request, env_helper) diff --git a/code/tests/functional/tests/backend_api/with_byod/test_conversation_flow.py b/code/tests/functional/tests/backend_api/with_byod/test_conversation_flow.py index 7aa24cb10..f2282e33e 100644 --- a/code/tests/functional/tests/backend_api/with_byod/test_conversation_flow.py +++ b/code/tests/functional/tests/backend_api/with_byod/test_conversation_flow.py @@ -47,16 +47,20 @@ def setup_default_mocking(httpserver: HTTPServer, app_config: AppConfig): httpserver.check() +@patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" +) @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" ) def test_azure_byod_responds_successfully_when_streaming( get_active_config_or_default_mock, + index_not_exists_mock, app_url: str, app_config: AppConfig, ): get_active_config_or_default_mock.return_value.prompts.conversational_flow = "byod" - + index_not_exists_mock.return_value = False # when response = requests.post(f"{app_url}{path}", json=body) @@ -92,17 +96,21 @@ def test_azure_byod_responds_successfully_when_streaming( } +@patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" +) @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" ) def test_post_makes_correct_call_to_azure_openai( get_active_config_or_default_mock, + index_not_exists_mock, app_url: str, app_config: AppConfig, httpserver: HTTPServer, ): get_active_config_or_default_mock.return_value.prompts.conversational_flow = "byod" - + index_not_exists_mock.return_value = False # when requests.post(f"{app_url}{path}", json=body) diff --git a/code/tests/test_app.py b/code/tests/test_app.py index 776e48bef..24dcfc9eb 100644 --- a/code/tests/test_app.py +++ b/code/tests/test_app.py @@ -84,7 +84,6 @@ def env_helper_mock(): ) env_helper.SHOULD_STREAM = True env_helper.is_auth_type_keys.return_value = True - env_helper.should_use_data.return_value = True env_helper.CONVERSATION_FLOW = ConversationFlow.CUSTOM.value yield env_helper @@ -599,6 +598,9 @@ def setup_method(self): ), ] + @patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" + ) @patch("create_app.AzureOpenAI") @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" @@ -611,6 +613,7 @@ def test_conversation_azure_byod_returns_correct_response_when_streaming_with_da generate_container_sas_mock: MagicMock, get_active_config_or_default_mock, azure_openai_mock: MagicMock, + index_not_exists_mock, env_helper_mock: MagicMock, client: FlaskClient, ): @@ -625,6 +628,7 @@ def test_conversation_azure_byod_returns_correct_response_when_streaming_with_da "byod" ) generate_container_sas_mock.return_value = "mock-sas" + index_not_exists_mock.return_value = False # when response = client.post( @@ -694,6 +698,9 @@ def test_conversation_azure_byod_returns_correct_response_when_streaming_with_da }, ) + @patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" + ) @patch("create_app.AzureOpenAI") @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" @@ -706,6 +713,7 @@ def test_conversation_azure_byod_returns_correct_response_when_streaming_with_da generate_container_sas_mock: MagicMock, get_active_config_or_default_mock, azure_openai_mock: MagicMock, + index_not_exists_mock, env_helper_mock: MagicMock, client: FlaskClient, ): @@ -720,6 +728,7 @@ def test_conversation_azure_byod_returns_correct_response_when_streaming_with_da openai_client_mock.chat.completions.create.return_value = ( self.mock_streamed_response ) + index_not_exists_mock.return_value = False # when response = client.post( @@ -755,6 +764,9 @@ def test_conversation_azure_byod_returns_correct_response_when_streaming_with_da "type": "system_assigned_managed_identity", } + @patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" + ) @patch("create_app.AzureOpenAI") @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" @@ -767,6 +779,7 @@ def test_conversation_azure_byod_returns_correct_response_when_not_streaming_wit generate_container_sas_mock: MagicMock, get_active_config_or_default_mock, azure_openai_mock: MagicMock, + index_not_exists_mock, env_helper_mock: MagicMock, client: FlaskClient, ): @@ -777,7 +790,7 @@ def test_conversation_azure_byod_returns_correct_response_when_not_streaming_wit "byod" ) generate_container_sas_mock.return_value = "mock-sas" - + index_not_exists_mock.return_value = False openai_client_mock = azure_openai_mock.return_value openai_client_mock.chat.completions.create.return_value = self.mock_response @@ -878,6 +891,9 @@ def test_conversation_azure_byod_returns_500_when_internalservererror_occurs( "administrator." } + @patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" + ) @patch("create_app.conversation_with_data") @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" @@ -886,6 +902,7 @@ def test_conversation_azure_byod_returns_429_on_rate_limit_error( self, get_active_config_or_default_mock, conversation_with_data_mock, + index_not_exists_mock, client, ): """Test that a 429 response is returned on RateLimitError for BYOD conversation.""" @@ -908,6 +925,7 @@ def test_conversation_azure_byod_returns_429_on_rate_limit_error( get_active_config_or_default_mock.return_value.prompts.conversational_flow = ( "byod" ) + index_not_exists_mock.return_value = False # when response = client.post( @@ -923,6 +941,9 @@ def test_conversation_azure_byod_returns_429_on_rate_limit_error( "Please wait a moment and try again." } + @patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" + ) @patch("create_app.AzureOpenAI") @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" @@ -931,17 +952,17 @@ def test_conversation_azure_byod_returns_correct_response_when_not_streaming_wit self, get_active_config_or_default_mock, azure_openai_mock, + index_not_exists_mock, env_helper_mock, client, ): """Test that the Azure BYOD conversation endpoint returns the correct response.""" # given - env_helper_mock.should_use_data.return_value = False env_helper_mock.SHOULD_STREAM = False get_active_config_or_default_mock.return_value.prompts.conversational_flow = ( "byod" ) - + index_not_exists_mock.return_value = True openai_client_mock = MagicMock() azure_openai_mock.return_value = openai_client_mock @@ -997,6 +1018,9 @@ def test_conversation_azure_byod_returns_correct_response_when_not_streaming_wit stream=False, ) + @patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" + ) @patch("create_app.AzureOpenAI") @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" @@ -1005,18 +1029,19 @@ def test_conversation_azure_byod_returns_correct_response_when_not_streaming_wit self, get_active_config_or_default_mock, azure_openai_mock, + index_not_exists_mock, env_helper_mock, client, ): """Test that the Azure BYOD conversation endpoint returns the correct response.""" # given - env_helper_mock.should_use_data.return_value = False env_helper_mock.SHOULD_STREAM = False env_helper_mock.AZURE_AUTH_TYPE = "rbac" env_helper_mock.AZURE_OPENAI_STOP_SEQUENCE = "" get_active_config_or_default_mock.return_value.prompts.conversational_flow = ( "byod" ) + index_not_exists_mock.return_value = True openai_client_mock = MagicMock() azure_openai_mock.return_value = openai_client_mock @@ -1073,6 +1098,9 @@ def test_conversation_azure_byod_returns_correct_response_when_not_streaming_wit stream=False, ) + @patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" + ) @patch("create_app.AzureOpenAI") @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" @@ -1081,16 +1109,16 @@ def test_conversation_azure_byod_returns_correct_response_when_streaming_without self, get_active_config_or_default_mock, azure_openai_mock, + index_not_exists_mock, env_helper_mock, client, ): """Test that the Azure BYOD conversation endpoint returns the correct response.""" # given - env_helper_mock.should_use_data.return_value = False get_active_config_or_default_mock.return_value.prompts.conversational_flow = ( "byod" ) - + index_not_exists_mock.return_value = True openai_client_mock = MagicMock() azure_openai_mock.return_value = openai_client_mock @@ -1120,6 +1148,9 @@ def test_conversation_azure_byod_returns_correct_response_when_streaming_without == '{"id": "response.id", "model": "mock-openai-model", "created": 0, "object": "response.object", "choices": [{"messages": [{"role": "assistant", "content": "mock content"}]}]}\n' ) + @patch( + "backend.batch.utilities.search.azure_search_handler.AzureSearchHelper._index_not_exists" + ) @patch("create_app.AzureOpenAI") @patch( "backend.batch.utilities.helpers.config.config_helper.ConfigHelper.get_active_config_or_default" @@ -1132,6 +1163,7 @@ def test_conversation_azure_byod_uses_semantic_config( generate_container_sas_mock: MagicMock, get_active_config_or_default_mock, azure_openai_mock: MagicMock, + index_not_exists_mock, client: FlaskClient, ): """Test that the Azure BYOD conversation endpoint uses the semantic configuration.""" @@ -1144,7 +1176,7 @@ def test_conversation_azure_byod_uses_semantic_config( openai_client_mock.chat.completions.create.return_value = ( self.mock_streamed_response ) - + index_not_exists_mock.return_value = False # when response = client.post( "/api/conversation",