Flash an alert when loqus instance is not reachable #4673

northwestwitch · 2024-06-12T07:15:17Z

This PR adds a functionality or fixes a bug.

Closes Warnings when loqusdb-api is not reachable #4669

Testing on cg-vm1 server (Clinical Genomics Stockholm)

Prepare for testing

Make sure the PR is pushed and available on Docker Hub
Fist book your testing time using the Pax software available at https://pax.scilifelab.se/. The resource you are going to call dibs on is scout-stage and the server is cg-vm1.
ssh <USER.NAME>@cg-vm1.scilifelab.se
sudo -iu hiseq.clinical
ssh localhost
(optional) Find out which scout branch is currently deployed on cg-vm1: podman ps
Stop the service with current deployed branch: systemctl --user stop scout.target
Start the scout service with the branch to test: systemctl --user start scout@<this_branch>
Make sure the branch is deployed: systemctl --user status scout.target
After testing is done, repeat procedure at https://pax.scilifelab.se/, which will release the allocated resource (scout-stage) to be used for testing by other users.

Testing on hasta server (Clinical Genomics Stockholm)

Prepare for testing

ssh <USER.NAME>@hasta.scilifelab.se
Book your testing time using the Pax software. us; paxa -u <user> -s hasta -r scout-stage. You can also use the WSGI Pax app available at https://pax.scilifelab.se/.
(optional) Find out which scout branch is currently deployed on cg-vm1: conda activate S_scout; pip freeze | grep scout-browser
Deploy the branch to test: bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_scout -t scout -b <this_branch>
Make sure the branch is deployed: us; scout --version
After testing is done, repeat the paxa procedure, which will release the allocated resource (scout-stage) to be used for testing by other users.

How to test -- on cg-vm1:

Change the settings of a scout institute to connect to a broken or not existent loqus instance
Go to a variant page

How to test -- locally:

Uncomment the lines in the config file relative to the loqusdb API instance
Go to a variant page

Expected outcome:

You should see a flashed error warning

Review:

code approved by DNIL
tests executed by DNIL, CR

dnil · 2024-06-12T07:22:40Z

Nice! Just ping when ready for review!

codecov · 2024-06-12T07:27:51Z

Codecov Report

Attention: Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.50%. Comparing base (5e7adeb) to head (2d59d03).

Files	Patch %	Lines
scout/server/blueprints/variant/controllers.py	60.00%	2 Missing ⚠️
scout/server/extensions/loqus_extension.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4673   +/-   ##
=======================================
  Coverage   84.49%   84.50%           
=======================================
  Files         310      310           
  Lines       18798    18797    -1     
=======================================
  Hits        15884    15884           
+ Misses       2914     2913    -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dnil

Nice start, but doesn't quite work yet?

default, stage-loqusdb-rd-api is very sparsely populated and returns no variant found for this variant.

scout/server/blueprints/variant/controllers.py

CHANGELOG.md

scout/server/blueprints/variant/controllers.py

scout/server/extensions/loqus_extension.py

Co-authored-by: Daniel Nilsson <daniel.k.nilsson@gmail.com>

northwestwitch · 2024-06-12T09:26:13Z

Ok I got confused by the fact that loqusdb api returns 404 when a variant is not found (I was expecting 200 but {} as content) and also from scout that modifies the response from that api before parsing it 🙄

Now it should be always returning {}, regardless of if the variant is found or not. But should flash a message if there is a connection error

scout/server/extensions/loqus_extension.py

dnil · 2024-06-12T11:02:56Z

scout/server/extensions/loqus_extension.py

-            LOG.info(search_resp.get("detail"))
+            if "details" not in search_resp.get("message", {}):  # Connection error
+                flash(
+                    f"Connection to Loqusdb instance returned error: '{search_resp['message']}'",


😊 Yeah, {} will not have "message"..

northwestwitch · 2024-06-12T11:57:11Z

tests/server/extensions/test_loqusdb_api_extension.py

@@ -78,15 +76,15 @@ def mockapi(*args):
 def test_loqus_api_snv_variant_not_found(loqus_api_app, monkeypatch, loqus_api_variant):
    # GIVEN a mocked loqus API that doesn't return usable info
    def mockapi(*args):
-        return {"message": {"details": "not found"}}


This was wrong, because api_get returns a response body with "message" only when there is an exception but variant not found doesn't trigger an exception

Right, that is loqusdbapi insert style errors. Maybe there was an intention to change at some point. 😊
Or fails obviously, which we were after elsewhere here.

sonarcloud · 2024-06-12T12:02:44Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

northwestwitch · 2024-06-12T12:04:40Z

scout/server/extensions/loqus_extension.py

-            return {}
-        return search_resp.get("content")
+
+        if search_resp.get("status_code") == 200:


This can't trigger error any more no?

dnil

Agreed, this solves the issue. I think I will make a new one for loqusdbapi to return a bit more elegantly, especially with totals as well even if the variant was not found, and we can revisit the issue. CLI loqusdb does return totals always, if asked to.

dnil · 2024-06-12T13:30:30Z

scout/server/blueprints/variant/controllers.py

-                obs_data[loqus_id]["observations"] = 0
+
+        if obs_data[loqus_id] == {}:  # Variant was not found
+            obs_data[loqus_id]["observations"] = 0


Yes, that should be it. I do think before long we should modify the loqusdbapi return so that we get a total set also for variant not found - after all we do have the variant type, and can tell how many of those cases we have - but that is not for this issue.

dnil · 2024-06-12T13:33:02Z

tests/server/extensions/test_loqusdb_api_extension.py

@@ -78,15 +76,15 @@ def mockapi(*args):
 def test_loqus_api_snv_variant_not_found(loqus_api_app, monkeypatch, loqus_api_variant):
    # GIVEN a mocked loqus API that doesn't return usable info
    def mockapi(*args):
-        return {"message": {"details": "not found"}}


Right, that is loqusdbapi insert style errors. Maybe there was an intention to change at some point. 😊
Or fails obviously, which we were after elsewhere here.

Chiara Rasi and others added 5 commits June 12, 2024 09:07

Clearer error message when scout fails to connect to a loqusdb instance

1f3c4d8

Forgot to add the files

0593473

Fix code style issues with Black

9c393fd

Small fixes

b71ddcb

Fix test accordingly

703361e

Chiara Rasi added 2 commits June 12, 2024 09:25

Removed unused imports

e0f1430

Assert is None

92f9d6b

northwestwitch requested a review from dnil June 12, 2024 07:31

dnil reviewed Jun 12, 2024

View reviewed changes

scout/server/blueprints/variant/controllers.py Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

scout/server/blueprints/variant/controllers.py Show resolved Hide resolved

scout/server/extensions/loqus_extension.py Outdated Show resolved Hide resolved

northwestwitch and others added 3 commits June 12, 2024 10:23

Update CHANGELOG.md

3537587

Co-authored-by: Daniel Nilsson <daniel.k.nilsson@gmail.com>

Fix to return {}, but alert in extension

a4f905e

Remove optional

583382d

Cover the case when instance cannot be found any more

922bb61

dnil reviewed Jun 12, 2024

View reviewed changes

Chiara Rasi added 4 commits June 12, 2024 13:26

Disregard error codes

fd9dca0

Adjust test

1dc7976

Nope, revert test, even if it doesn't pass

2c38981

Fix test for good

3db1465

northwestwitch commented Jun 12, 2024

View reviewed changes

Simplify

2d59d03

northwestwitch commented Jun 12, 2024

View reviewed changes

dnil approved these changes Jun 12, 2024

View reviewed changes

northwestwitch merged commit 535a38b into main Jun 13, 2024
25 checks passed

northwestwitch deleted the loqus_timeout branch July 24, 2024 08:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flash an alert when loqus instance is not reachable #4673

Flash an alert when loqus instance is not reachable #4673

northwestwitch commented Jun 12, 2024 •

edited

Loading

dnil commented Jun 12, 2024

codecov bot commented Jun 12, 2024 •

edited

Loading

dnil left a comment

northwestwitch commented Jun 12, 2024 •

edited

Loading

dnil Jun 12, 2024

northwestwitch Jun 12, 2024

dnil Jun 12, 2024

sonarcloud bot commented Jun 12, 2024

northwestwitch Jun 12, 2024 •

edited

Loading

dnil left a comment

dnil Jun 12, 2024

dnil Jun 12, 2024

Flash an alert when loqus instance is not reachable #4673

Flash an alert when loqus instance is not reachable #4673

Conversation

northwestwitch commented Jun 12, 2024 • edited Loading

dnil commented Jun 12, 2024

codecov bot commented Jun 12, 2024 • edited Loading

Codecov Report

dnil left a comment

Choose a reason for hiding this comment

northwestwitch commented Jun 12, 2024 • edited Loading

dnil Jun 12, 2024

Choose a reason for hiding this comment

northwestwitch Jun 12, 2024

Choose a reason for hiding this comment

dnil Jun 12, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Jun 12, 2024

Quality Gate passed

northwestwitch Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

dnil left a comment

Choose a reason for hiding this comment

dnil Jun 12, 2024

Choose a reason for hiding this comment

dnil Jun 12, 2024

Choose a reason for hiding this comment

northwestwitch commented Jun 12, 2024 •

edited

Loading

codecov bot commented Jun 12, 2024 •

edited

Loading

northwestwitch commented Jun 12, 2024 •

edited

Loading

northwestwitch Jun 12, 2024 •

edited

Loading