Skip to content

[CV2-6020] test updates and handling for missing min_es_score #485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Feb 7, 2025
Merged

[CV2-6020] test updates and handling for missing min_es_score #485

merged 2 commits into from
Feb 7, 2025

Conversation

skyemeedan
Copy link
Contributor

@skyemeedan skyemeedan commented Feb 5, 2025 •
edited
Loading

Description

adds more test around min_es_score behavior, updates to

For updating the behavior, seem the goal would be
min_es_score missing or None: set to zero with warning
min_es_score cannot parse as float: log error and raise exception?

Reference: https://meedan.atlassian.net/browse/CV2-6020

How has this been tested?

Test coverage added, ran locally,

  • confirmed warning logged appropriately from test WARNING:flask.app:min_es_score is missing or None, defaulting to 0.0
  • confirmed Exception raised by float parsing is logged appropriately ERROR:flask.app:Unable to parse min_es_score 'fifty':could not convert string to float: 'fifty'
  • assertion checks that error case results in 500

Have you considered secure coding practices when writing this code?

no new security concerns

DGaffney
DGaffney approved these changes Feb 7, 2025
View reviewed changes
Copy link
Contributor

@DGaffney DGaffney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was running into the same problems on CV2-6022 - this looks basically correct!

@skyemeedan skyemeedan merged commit 41c9f8e into develop Feb 7, 2025
4 checks passed
skyemeedan pushed a commit that referenced this pull request Mar 3, 2025
v0.187.2 Squashed commit of the following from develop:
4eb96c4 
commit 008f4f2
Author: ahmednasserswe <ahmed.nasser.swe@gmail.com>
Date:   Fri Feb 28 06:58:11 2025 +0100

    CV2-5077-Add-test-coverage-for-OpenAI-module-in-Alegre (#491)

    * create test_openai.py and add `test_openai_get_document_body`
    * Update test_openai.py with `test_retrieve_openai_embeddings_handles_api_error` and`test_retrieve_openai_embeddings_calls_openai_api`
    * Update test_openai.py to mock redis cache
    * Update test_openai.py to include tests that call the api
    * mocking cache in `test_openai_get_document_body` and `test_retrieve_openai_embeddings_calls_openai_api`
    * asserting openai key is set during CI
    * adding a unit test to find if openai_api_key is in env
    * added test OPENAI key for CI to encoded .env file https://meedan.atlassian.net/wiki/spaces/ENG/pages/1942945794/How+to+encrypt+and+decrypt+.enc+files+in+Alegre

    ---------

    Co-authored-by: Skye Bender-deMoll <skye@meedan.org>

commit d63cc85
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Thu Feb 27 15:48:30 2025 -0800

    update to show checkweb direct dependency on checkApi

commit 60be6ac
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Wed Feb 26 14:39:37 2025 -0800

    added links to new docs in README, moved png files from toplevel into /doc

commit 3636375
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Wed Feb 26 14:28:44 2025 -0800

    added explanation and API dependencies

commit cb7e5b1
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Wed Feb 26 14:22:51 2025 -0800

    renamed and corrected image links

commit 9412b7a
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Wed Feb 26 12:46:38 2025 -0800

    [CV2-5885] first sketch of infra based on notes from Devin

commit 7b135a2
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Thu Feb 27 16:16:11 2025 -0800

    adding --threads back in

commit 4ea58a1
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Thu Feb 27 15:09:00 2025 -0800

    [CV2-6066] doubling worker processes from 8 to 16, and removing --threads since docs show it is only for gevent

commit 94dd470
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Thu Feb 27 11:12:59 2025 -0800

    doubling threads again from 8 to 16

commit 227fa79
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Thu Feb 27 11:11:42 2025 -0800

    disabling some tests to simplify testing protocol

commit 1afea6a
Author: Skye Bender-deMoll <skye@meedan.org>
Date:   Thu Feb 27 11:03:58 2025 -0800

    [CV2-6066] added a load test for async endpoint

commit cf3e2c2
Author: Martin Peck <51542678+sonoransun@users.noreply.github.com>
Date:   Wed Feb 26 08:25:46 2025 -0700

    Fix ecs deploy dependencies with updated version pins. (#493)

    * Try to resolve dependency errors in ecs deploy by updating botocore, boto3, and awscli version pins.

commit c0930a9
Author: Devin Gaffney <itsme@devingaffney.com>
Date:   Tue Feb 25 20:10:14 2025 -0500

    CV2-6022 update tests to use non-deprecated URLs (#487)

    * CV2-6022 update tests to use non-deprecated (presto backed) URLs
    * update queries to no longer use old endpoint
    * test for min_es_score fix
    * add custom error
    * add sleeps, it ended up just being eventual consistency
    * switch to greaters, deprecated test file having weird interaction effects that we probably don't need to solve for right now
    * Update app/test/test_similarity.py

commit 6645936
Author: Skye Bender-deMoll <122867176+skyemeedan@users.noreply.github.com>
Date:   Tue Feb 25 11:00:33 2025 -0800

    [CV2-6066] load tests 3 (#492)

    * [CV2-6066] change warning message to not include actual key value so sentry errors will group
    * double number of alegre web server worker process-THREADS to 8 in QA and Live to handle more connections
    ---------
    Co-authored-by: Skye Bender-deMoll <skye@meedan.org>

commit c4828f5
Author: Devin Gaffney <itsme@devingaffney.com>
Date:   Mon Feb 24 14:32:45 2025 -0800

    [CV2-6075] documentation for Check-Alegre-Presto media indexing and similarity workflows (#489)

    * CV2-6075 initial stab at documentation for Check-Alegre-Presto media indexing and similarity workflows

commit 3494d9e
Author: Skye Bender-deMoll <122867176+skyemeedan@users.noreply.github.com>
Date:   Fri Feb 14 09:06:34 2025 -0800

    [CV2-6066] load tests for text similarity endpoint (#490)

    * adding faker pip requirement for random text generation for benchmarks (testing only)
    * [CV2-6066] draft load testing script for text similarity endpoints
    * updates to benchmark (disabling parallel inserts) and some code comments
    * re-enabled parallel tests, reducing so all tests only run 100 items
    * added check to not run in live
    ---------
    Co-authored-by: Skye Bender-deMoll <skye@meedan.org>

commit 41c9f8e
Author: Skye Bender-deMoll <122867176+skyemeedan@users.noreply.github.com>
Date:   Fri Feb 7 08:45:44 2025 -0800

    [CV2-6020] test updates and handling for missing min_es_score (#485)

    * [CV2-6020] test updates and handling for missing min_es_score
    * added more explicit error messaging and confirmed error status code, corrected failing test
    ---------
    Co-authored-by: Skye Bender-deMoll <skye@meedan.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@computermacgyver computermacgyver Awaiting requested review from computermacgyver computermacgyver is a code owner

1 more reviewer

@DGaffney DGaffney DGaffney approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@skyemeedan @DGaffney