From 6c085a25e2c5967c2e98569491694ce3d4b27df6 Mon Sep 17 00:00:00 2001 From: Dhruv Prasanna <39568064+21dhruvp@users.noreply.github.com> Date: Sat, 21 Oct 2023 02:04:22 -0400 Subject: [PATCH 1/5] Cleaned up error checking and throwing (#47) * Cleaned up error checking and throwing * fixed formatting errors and using is instead of '==' --- config/config.py | 4 ++-- config/env.py | 2 +- controllers/contract.py | 5 +++++ controllers/me.py | 9 ++++++++- database/base_db.py | 7 +------ database/users.py | 2 +- managers/contract.py | 4 ++-- services/docusign/docusign.py | 7 +++---- 8 files changed, 23 insertions(+), 17 deletions(-) diff --git a/config/config.py b/config/config.py index 8bb9f7d..4285f76 100644 --- a/config/config.py +++ b/config/config.py @@ -9,10 +9,10 @@ def __init__(self) -> None: def get_contract_limit(self, key: str) -> int: value = self._config['contract_limits'].get(key) - assert type(value) == int, "Invalid config file. Contract limits can only be integers." + assert type(value) is int, "Invalid config file. Contract limits can only be integers." return value def get_docusign_config(self, key: str) -> str: value = self._config['docusign'].get(key) - assert type(value) == str, "Invalid docusign config" + assert type(value) is str, "Invalid docusign config" return value diff --git a/config/env.py b/config/env.py index 6218a69..9259d9f 100644 --- a/config/env.py +++ b/config/env.py @@ -7,7 +7,7 @@ def load_env(input: str) -> str: data = os.getenv(input) - assert type(data) == str and len(data) > 0, f"Invalid or no '{input}' environment variable." + assert type(data) is str and len(data) > 0, f"Invalid or no '{input}' environment variable." return data diff --git a/controllers/contract.py b/controllers/contract.py index f7f8563..f69021a 100644 --- a/controllers/contract.py +++ b/controllers/contract.py @@ -56,6 +56,11 @@ async def post(self, item: PostItem, current_user: CognitoClaims = Depends(get_c status_code=status.HTTP_409_CONFLICT, detail='Cannot make contract since there is nobody to approve the contract' ) + except NotImplementedError: + raise HTTPException( + status_code=status.HTTP_501_NOT_IMPLEMENTED, + detail='Unable to create contract of specified type' + ) return JSONResponse( status_code=status.HTTP_200_OK, diff --git a/controllers/me.py b/controllers/me.py index 678e8ef..79c3315 100644 --- a/controllers/me.py +++ b/controllers/me.py @@ -11,6 +11,7 @@ from typing_extensions import TypedDict import uuid from database.users import UsersDB +from pymongo.errors import DuplicateKeyError class PostItem(BaseModel): @@ -70,7 +71,13 @@ async def patch(self, current_user: CognitoClaims = Depends(get_current_user)) - ) async def post(self, item: PostItem, current_user: CognitoClaims = Depends(get_current_user)) -> Response: # type: ignore[no-any-unimported] - ret: bool = await MeManager().create_user(current_user.sub, str(item.vendor_type)) + try: + ret: bool = await MeManager().create_user(current_user.sub, str(item.vendor_type)) + except DuplicateKeyError: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="Specified user already exists" + ) if not ret: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, diff --git a/database/base_db.py b/database/base_db.py index edbd667..6de7e36 100644 --- a/database/base_db.py +++ b/database/base_db.py @@ -1,6 +1,5 @@ from motor.motor_asyncio import AsyncIOMotorClient, AsyncIOMotorDatabase, AsyncIOMotorCollection from config.env import MONGO_URI, MONGO_DB_NAME -import logging from typing import Optional, List from utilities.types import MongoMappingType, JSONDict @@ -18,11 +17,7 @@ def _get_client(cls) -> AsyncIOMotorClient: # type: ignore[no-any-unimported] @classmethod def _verify_connection(cls, client: AsyncIOMotorClient) -> None: # type: ignore[no-any-unimported] - try: - client.server_info() - except Exception as e: - logging.critical("Cannot connect to db.") - raise e + client.server_info() @classmethod def get_database(cls) -> AsyncIOMotorDatabase: # type: ignore[no-any-unimported] diff --git a/database/users.py b/database/users.py index 72101c9..1bcf10c 100644 --- a/database/users.py +++ b/database/users.py @@ -64,7 +64,7 @@ async def _get_random_reviewer(cls, role: str) -> Optional[MongoMappingType]: results = await cls.get_random(cls.get_collection(), 1, query) if len(results) == 0: return None - assert len(results) == 1 + assert len(results) == 1, "Recieved too many results" return results[0] @classmethod diff --git a/managers/contract.py b/managers/contract.py index e8faeac..2792a26 100644 --- a/managers/contract.py +++ b/managers/contract.py @@ -34,8 +34,8 @@ async def create_contract( approver_email = "test@gmail.com" approver_name = "test" - assert type(approver_email) == str - assert type(approver_name) == str + assert type(approver_email) is str, "Expected a string for approver email" + assert type(approver_name) is str, "Expected a string for approver name" data = ContractData( num_additional_chairs=num_additional_chairs, diff --git a/services/docusign/docusign.py b/services/docusign/docusign.py index 65d83a7..615c0aa 100644 --- a/services/docusign/docusign.py +++ b/services/docusign/docusign.py @@ -1,4 +1,3 @@ -import sys from .envelope import Contract from services.docusign import ContractData from docusign_esign import ApiClient @@ -61,7 +60,7 @@ def _handle_consent(cls, err: ApiException, callback: Callable[[ApiClient, str, return callback(api_client, private_key, contract_data) logging.error(body) - sys.exit("Failed to grant consent") # TODO can this sys.exit be removed? + raise Exception("Consent could not be aquired") @classmethod def _run(cls, api_client: ApiClient, private_key: str, contract_data: ContractData) -> str: # type: ignore[no-any-unimported] @@ -74,11 +73,11 @@ def _run(cls, api_client: ApiClient, private_key: str, contract_data: ContractDa envelope_data: Dict[str, str] = Contract(access_token, base_path, account_id).make_contract(contract_data) - assert type(envelope_data) == dict, f"Invalid envelope response: {envelope_data}" + assert type(envelope_data) is dict, f"Invalid envelope response: {envelope_data}" assert "envelope_id" in envelope_data, f"Envelope does not contain ID: {envelope_data}" envelope_id: Optional[str] = envelope_data["envelope_id"] - assert type(envelope_id) == str, f"Invalid envelope id {envelope_id} of type {type(envelope_id)}" + assert type(envelope_id) is str, f"Invalid envelope id {envelope_id} of type {type(envelope_id)}" return envelope_id From 708ce2e8a7de2d43069ecac768eb12c5c3e5c986 Mon Sep 17 00:00:00 2001 From: Eric Zhu Date: Wed, 25 Oct 2023 01:44:51 -0400 Subject: [PATCH 2/5] add ci --- .github/workflow/ci.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .github/workflow/ci.yaml diff --git a/.github/workflow/ci.yaml b/.github/workflow/ci.yaml new file mode 100644 index 0000000..28c7121 --- /dev/null +++ b/.github/workflow/ci.yaml @@ -0,0 +1,15 @@ +name: Main workflow + +on: + pull_request: + push: + +jobs: + runs-on: ubuntu-latest + if: github.event.pull_request.draft == false + + steps: + - name: make check + run: | + pip install -r requirements.txt + make check From 6558488de9ea620797ead4bd32cea6c9da034bc6 Mon Sep 17 00:00:00 2001 From: Eric Zhu Date: Wed, 25 Oct 2023 01:45:48 -0400 Subject: [PATCH 3/5] fix ci --- .github/{workflow => workflows}/ci.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/{workflow => workflows}/ci.yaml (100%) diff --git a/.github/workflow/ci.yaml b/.github/workflows/ci.yaml similarity index 100% rename from .github/workflow/ci.yaml rename to .github/workflows/ci.yaml From 976839e393236e3b3379c4c6f442fa95c1817ed8 Mon Sep 17 00:00:00 2001 From: Eric Zhu Date: Wed, 25 Oct 2023 01:46:30 -0400 Subject: [PATCH 4/5] fix ci --- .github/workflows/ci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 28c7121..7995fed 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -5,6 +5,7 @@ on: push: jobs: + test: runs-on: ubuntu-latest if: github.event.pull_request.draft == false From 62a3a92ef5b0c38e68898d273dec11daf49bade6 Mon Sep 17 00:00:00 2001 From: Eric Zhu Date: Wed, 25 Oct 2023 01:47:12 -0400 Subject: [PATCH 5/5] fix ci --- .github/workflows/ci.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7995fed..a6c5ee5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -10,6 +10,10 @@ jobs: if: github.event.pull_request.draft == false steps: + - name: Checkout + uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.sha }} - name: make check run: | pip install -r requirements.txt