From 2d802d14093ce283812e7d8c998d64078c204644 Mon Sep 17 00:00:00 2001 From: Ross Smith Date: Wed, 24 Jul 2024 08:45:31 +0100 Subject: [PATCH] build: Add linting to the PR process (#1168) --- .github/workflows/tests.yml | 2 + code/create_app.py | 3 +- .../test_azure_search_handler.py | 9 +-- code/tests/test_app.py | 56 ++++++++++--------- .../test_azure_computer_vision_client.py | 4 +- 5 files changed, 41 insertions(+), 33 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 903d035ef..890c1e543 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -95,3 +95,5 @@ jobs: cache-dependency-path: "code/frontend/package-lock.json" - name: Run frontend unit tests run: make unittest-frontend + - name: Lint + run: make lint diff --git a/code/create_app.py b/code/create_app.py index a64b2245a..08b9f254a 100644 --- a/code/create_app.py +++ b/code/create_app.py @@ -9,7 +9,7 @@ from os import path import sys import requests -from openai import AzureOpenAI, Stream, RateLimitError, APIStatusError +from openai import AzureOpenAI, Stream, APIStatusError from openai.types.chat import ChatCompletionChunk from flask import Flask, Response, request, Request, jsonify from dotenv import load_dotenv @@ -24,6 +24,7 @@ ERROR_GENERIC_MESSAGE = "An error occurred. Please try again. If the problem persists, please contact the site administrator." logger = logging.getLogger(__name__) + def stream_with_data(response: Stream[ChatCompletionChunk]): """This function streams the response from Azure OpenAI with data.""" response_obj = { diff --git a/code/tests/search_utilities/test_azure_search_handler.py b/code/tests/search_utilities/test_azure_search_handler.py index ca929f24d..8a9f0d2f7 100644 --- a/code/tests/search_utilities/test_azure_search_handler.py +++ b/code/tests/search_utilities/test_azure_search_handler.py @@ -364,11 +364,13 @@ def test_semantic_search_with_advanced_image_processing( top=handler.env_helper.AZURE_SEARCH_TOP_K, ) + @pytest.fixture def search_handler(): - env_helper=Mock() + env_helper = Mock() return AzureSearchHandler(env_helper) + def test_search_with_facets_no_search_client(search_handler): search_handler.search_client = None @@ -387,12 +389,11 @@ def test_search_with_facets_valid(search_handler): assert result == "search_results" - def test_get_unique_files_no_results(search_handler): results = None facet_key = "facet_key" result = search_handler.get_unique_files(results, facet_key) - assert(result, []) + assert result == [] def test_get_unique_files_with_results(search_handler): @@ -405,7 +406,7 @@ def test_get_unique_files_with_results(search_handler): } facet_key = "facet_key" result = search_handler.get_unique_files(mock_results, facet_key) - assert(result, ["file1", "file2"]) + assert result == ["file1", "file2"] def test_delete_from_index(handler, mock_search_client): diff --git a/code/tests/test_app.py b/code/tests/test_app.py index b38c64532..3f6e18866 100644 --- a/code/tests/test_app.py +++ b/code/tests/test_app.py @@ -4,7 +4,6 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch, ANY -from httpx import Response from openai import RateLimitError, BadRequestError, InternalServerError import pytest from flask.testing import FlaskClient @@ -335,17 +334,19 @@ def test_conversation_custom_returns_error_response_on_rate_limit_error( response_mock = Mock() response_mock.status_code = 429 response_mock.json.return_value = { - 'error': { - 'code': "429", - 'message': 'Requests to the Embeddings_Create Operation under Azure OpenAI API version 2024-02-01 ' - 'have exceeded call rate limit of your current OpenAI S0 pricing tier. Please retry after ' - '2 seconds. Please go here: https://aka.ms/oai/quotaincrease if you would like to further ' - 'increase the default rate limit.' + "error": { + "code": "429", + "message": "Requests to the Embeddings_Create Operation under Azure OpenAI API version 2024-02-01 " + "have exceeded call rate limit of your current OpenAI S0 pricing tier. Please retry after " + "2 seconds. Please go here: https://aka.ms/oai/quotaincrease if you would like to further " + "increase the default rate limit.", } } body_mock = {"error": "Rate limit exceeded"} - rate_limit_error = RateLimitError("Rate limit exceeded", response=response_mock, body=body_mock) + rate_limit_error = RateLimitError( + "Rate limit exceeded", response=response_mock, body=body_mock + ) get_orchestrator_config_mock.side_effect = rate_limit_error # when @@ -359,18 +360,20 @@ def test_conversation_custom_returns_error_response_on_rate_limit_error( assert response.status_code == 429 assert response.json == { "error": "We're currently experiencing a high number of requests for the service you're trying to access. " - "Please wait a moment and try again." + "Please wait a moment and try again." } @patch("create_app.get_orchestrator_config") def test_conversation_custom_returns_500_when_internalservererror_occurs( - self, get_orchestrator_config_mock, env_helper_mock, client + self, get_orchestrator_config_mock, env_helper_mock, client ): """Test that an error response is returned when an exception occurs.""" # given response_mock = MagicMock() response_mock.status_code = 500 - get_orchestrator_config_mock.side_effect = InternalServerError("Test exception", response=response_mock, body="") + get_orchestrator_config_mock.side_effect = InternalServerError( + "Test exception", response=response_mock, body="" + ) # when response = client.post( @@ -383,7 +386,7 @@ def test_conversation_custom_returns_500_when_internalservererror_occurs( assert response.status_code == 500 assert response.json == { "error": "An error occurred. Please try again. If the problem persists, please contact the site " - "administrator." + "administrator." } @patch("create_app.get_message_orchestrator") @@ -756,13 +759,15 @@ def test_conversation_azure_byod_returns_500_when_exception_occurs( @patch("create_app.conversation_with_data") def test_conversation_azure_byod_returns_500_when_internalservererror_occurs( - self, conversation_with_data_mock, env_helper_mock, client + self, conversation_with_data_mock, env_helper_mock, client ): """Test that an error response is returned when an exception occurs.""" # given response_mock = MagicMock() response_mock.status_code = 500 - conversation_with_data_mock.side_effect = InternalServerError("Test exception", response=response_mock, body="") + conversation_with_data_mock.side_effect = InternalServerError( + "Test exception", response=response_mock, body="" + ) env_helper_mock.CONVERSATION_FLOW = ConversationFlow.BYOD.value # when @@ -776,29 +781,30 @@ def test_conversation_azure_byod_returns_500_when_internalservererror_occurs( assert response.status_code == 500 assert response.json == { "error": "An error occurred. Please try again. If the problem persists, please contact the site " - "administrator." + "administrator." } @patch("create_app.conversation_with_data") def test_conversation_azure_byod_returns_429_on_rate_limit_error( - self, conversation_with_data_mock, env_helper_mock, client + self, conversation_with_data_mock, env_helper_mock, client ): """Test that a 429 response is returned on RateLimitError for BYOD conversation.""" # given response_mock = MagicMock() response_mock.status_code = 400 response_mock.json.return_value = { - 'error': { - 'requestid': 'f30740e1-c6e1-48ab-ab1e-35469ed41ba4', - 'code': "400", - 'message': 'An error occurred when calling Azure OpenAI: Rate limit reached for AOAI embedding ' - 'resource: Server responded with status 429. Error message: {"error":{"code":"429",' - '"message": "Rate limit is exceeded. Try again in 44 seconds."}}' + "error": { + "requestid": "f30740e1-c6e1-48ab-ab1e-35469ed41ba4", + "code": "400", + "message": "An error occurred when calling Azure OpenAI: Rate limit reached for AOAI embedding " + 'resource: Server responded with status 429. Error message: {"error":{"code":"429",' + '"message": "Rate limit is exceeded. Try again in 44 seconds."}}', } } - conversation_with_data_mock.side_effect = BadRequestError(message="Error code: 400", response=response_mock, - body="") + conversation_with_data_mock.side_effect = BadRequestError( + message="Error code: 400", response=response_mock, body="" + ) env_helper_mock.CONVERSATION_FLOW = ConversationFlow.BYOD.value # when @@ -812,7 +818,7 @@ def test_conversation_azure_byod_returns_429_on_rate_limit_error( assert response.status_code == 429 assert response.json == { "error": "We're currently experiencing a high number of requests for the service you're trying to access. " - "Please wait a moment and try again." + "Please wait a moment and try again." } @patch("create_app.AzureOpenAI") diff --git a/code/tests/utilities/helpers/test_azure_computer_vision_client.py b/code/tests/utilities/helpers/test_azure_computer_vision_client.py index 67ae1ffd2..ff8c70b9e 100644 --- a/code/tests/utilities/helpers/test_azure_computer_vision_client.py +++ b/code/tests/utilities/helpers/test_azure_computer_vision_client.py @@ -5,8 +5,6 @@ import pytest from pytest_httpserver import HTTPServer from trustme import CA -import werkzeug -import time from requests import ReadTimeout from backend.batch.utilities.helpers.azure_computer_vision_client import ( @@ -178,7 +176,7 @@ def test_returns_text_vectors( @mock.patch("backend.batch.utilities.helpers.azure_computer_vision_client.requests") def test_vectorize_image_calls_computer_vision_timeout( - mock_requests: MagicMock, azure_computer_vision_client: AzureComputerVisionClient + mock_requests: MagicMock, azure_computer_vision_client: AzureComputerVisionClient ): mock_requests.post.side_effect = ReadTimeout("An error occurred") # when