UIP-47: Misc tidying #3558

ialarmedalien · 2024-06-07T20:47:23Z

Description of PR purpose/changes

Some miscellaneous fixes and so on that I made over the last day or so. Includes:

moved the python-requirements file into src/ so it's easier to update py dependencies;
edited the dockerfile so it transfers over only the files/folders that it uses, instead of copying over the whole repo; this also makes it quicker to build images locally as more steps can be cached;
updated some documentation;
added typing/brief docs to some python files.

Jira Ticket / Issue

Related Jira ticket: https://kbase-jira.atlassian.net/browse/UIP-47

Added the Jira Ticket to the title of the PR (e.g. DATAUP-69 Adds a PR template)

Testing Instructions

Details for how to test the PR:

Tests pass locally and in GitHub Actions
Changes available by spinning up a local narrative and navigating to X to see Y

Dev Checklist:

Updating Version and Release Notes (if applicable)

Version has been bumped for each release
Release notes have been updated for each release (and during the merge of feature branches)

…src/ folder for ease of updating, and updating the docs accordingly. also added some typing to some python files edited dockerfile to transfer over individual folders instead of copying the whole repo and creating an image with a load of unnecessary junk in it.

ialarmedalien · 2024-06-07T20:48:30Z

.github/dependabot.yml

- package-ecosystem: pip
-  directory: "/docker_image_dependencies"
-  schedule:
-    interval: monthly
-    time: '11:00'
-  groups:
-    docker-image-python-deps:
-      patterns:
-        "*"
-  open-pull-requests-limit: 20


all the requirements files are now in the /src directory, so 🤞 dependabot will be a bit more dependable in reporting updates

ialarmedalien · 2024-06-07T20:51:14Z

Dockerfile

+WORKDIR /kb/dev_container/narrative
+
+RUN mkdir -p /kb/deployment/ui-common/ && \
+    mkdir -p /tmp/narrative && \


just moved up from the end of this section

ialarmedalien · 2024-06-07T20:51:50Z

Dockerfile

    # Compile Javascript down into an itty-bitty ball unless SKIP_MINIFY is non-empty
    echo Skip="$SKIP_MINIFY" && \
    [ -n "$SKIP_MINIFY" ] || npm run minify && \
-    # install the narrative and jupyter console
+    # Generate a version file that we can scrape later
+    ./src/scripts/kb-update-config -f src/config.json.templ -o /kb/deployment/ui-common/narrative_version && \


moved a little later in the block for ease of development

ialarmedalien · 2024-06-07T20:52:31Z

RELEASE_NOTES.md

@@ -56,7 +56,7 @@ This is built on the Jupyter Notebook v6.5.6 and IPython 8.25.0 (more notes will
  - scikit-learn: 1.2.1 -> 1.5.0
  - statsmodels: 0.14.2 (new)
  - terminado: 0.17.1 -> 0.18.0
-  - tornado: 6.2 -> 6.4
+  - tornado: 6.2 -> 6.4.1


Of course there's another update just when I thought I had all the python stuff done...

ialarmedalien · 2024-06-07T20:53:07Z

docs/install/dockerhub_builds.md

@@ -1,30 +1,32 @@
 ## Building Narrative Images with Dockerhub
 (Updated 6/1/2018)

-We currently make use of Travis and Dockerhub (https://hub.docker.com) to manage Narrative production builds. There are three images here that stack on each other.
+We currently make use of GitHub Actions and the GitHub Packages (aka the GitHubContainer Repository, https://ghcr.io) to manage Narrative production builds. The narrative comprises two images (to facilitate reasonable narrative build times) and a third tiny image for the narrative version.


started updating this documentation; will do some more on it when I get this ui-assets thing sorted out

ialarmedalien · 2024-06-07T20:53:30Z

docs/install/dockerhub_builds.md


 ### How to build images
-Travis-CI is used to run regression testing on the branches declared in the branches stanza of the top level travis.yml file. Currently the travis file builds on these branches:
+
+GitHub Actions is used to run regression testing on the branches declared in the branches stanza of the top level travis.yml file. Currently the travis file builds on these branches:


Enthusiasm/energy for documentation ran out here.

ialarmedalien · 2024-06-07T20:53:47Z

scripts/build_narrative_container.sh

-NAR_BASE="kbase/narrbase"
-NAR_BASE_VER="7.0"


doesn't exist

Not anymore it doesn't!

ialarmedalien · 2024-06-07T20:54:27Z

scripts/build_narrative_container.sh

@@ -58,21 +56,6 @@ mkdir -p $WEBROOT_DIR
 ./src/scripts/kb-update-config -f src/config.json.templ -o $WEBROOT_DIR/narrative_version -e $env -d $dev_mode || exit 1
 cp kbase-extension/static/kbase/config/data_source_config.json $WEBROOT_DIR/data_source_config.json

-# Make sure the base image is there. If not, build it.


all refers to the narrbase image, which is no longer with us.

ialarmedalien · 2024-06-07T20:55:38Z

scripts/install_narrative_docker.sh

@@ -8,12 +8,10 @@
 PYTHON=python

 SCRIPT_TGT="kbase-narrative"
-SCRIPT_TGT2="headless-narrative"


this script exists but it points at a python file that no longer does ==> delete the code so that people aren't tempted to try it

ialarmedalien · 2024-06-07T20:55:56Z

src/biokbase/narrative/contents/kbasewsmanager.py

@@ -105,8 +106,8 @@ class KBaseWSManager(KBaseWSManagerMixin, ContentsManager):
    # a safety net
    wsid_regex = re.compile(r"[\W]+", re.UNICODE)

-    def __init__(self, *args, **kwargs):
-        """Verify that we can connect to the configured WS instance"""
+    def __init__(self: "KBaseWSManager", *args, **kwargs) -> None:


all typing changes here

ialarmedalien · 2024-06-07T20:56:18Z

src/biokbase/narrative/contents/kbasewsmanager.py

-    def dir_exists(self, path):
-        """If it's blank, just return True -
+    def dir_exists(self: "KBaseWSManager", path: str) -> bool:
+        """Returns a boolean indicating whether a directory exists.


getting fancy -- typing AND some basic documentation!

ialarmedalien · 2024-06-07T20:56:47Z

src/biokbase/narrative/contents/kbasewsmanager.py

-        nar_id = "ws.{}.obj.{}".format(nar["wsid"], nar["objid"])
-        model = base_model("{} - {} - {}".format(nar["saved_by"], nar_id, nar["name"]), nar_id)
+    def _wsobj_to_model(self: "KBaseWSManager", nar, content: bool = True) -> dict[str, Any]:
+        nar_id = f"ws.{nar['wsid']}.obj.{nar['objid']}"


use f"..." instead of string.format

ialarmedalien · 2024-06-07T20:57:41Z

src/biokbase/narrative/contents/manager_util.py

@@ -1,7 +1,9 @@
 """Utils for implementing the KBase Narrative manager."""

+from typing import Any


ialarmedalien · 2024-06-07T20:58:06Z

src/biokbase/narrative/contents/narrativeio.py

@@ -6,9 +6,11 @@
 import json
 import re
 from collections import Counter
+from typing import Any


typing additions

ialarmedalien · 2024-06-07T20:58:29Z

src/requirements.txt

@@ -9,4 +9,4 @@ pyyaml==6.0.1
 requests==2.32.3
 scikit-learn==1.5.0
 statsmodels==0.14.2
-tornado==6.4
+tornado==6.4.1


updated package

sonarcloud · 2024-06-07T21:02:03Z

Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarCloud

codecov · 2024-06-07T21:12:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 25.74%. Comparing base (aa67a64) to head (b5cf3ac).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3558      +/-   ##
===========================================
- Coverage    29.70%   25.74%   -3.97%     
===========================================
  Files          496      461      -35     
  Lines        50444    46694    -3750     
===========================================
- Hits         14984    12021    -2963     
+ Misses       35460    34673     -787

see 36 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b401562...b5cf3ac. Read the comment docs.

ialarmedalien · 2024-06-09T18:55:11Z

Dockerfile

 ADD ./deployment/ /kb/deployment/
-WORKDIR /kb/dev_container/narrative
+ADD ./build.js /kb/dev_container/narrative/


Copying over the directories and files that are actually used, instead of everything under the sun. Attempted to order them by approximate likelihood of being changed, although everything after nbextensions is a lot more likely to be edited than the stuff before it.

Good idea. Eventually (not this PR, and maybe not right away) it might be nice to make a production-only image and leave out tests, test data, test-only dependencies, and so on.

But this is a good step on that path!

briehl

👍

briehl · 2024-06-11T01:13:24Z

Dockerfile

 ADD ./deployment/ /kb/deployment/
-WORKDIR /kb/dev_container/narrative
+ADD ./build.js /kb/dev_container/narrative/


Good idea. Eventually (not this PR, and maybe not right away) it might be nice to make a production-only image and leave out tests, test data, test-only dependencies, and so on.

But this is a good step on that path!

briehl · 2024-06-11T01:14:31Z

RELEASE_NOTES.md

@@ -56,7 +56,7 @@ This is built on the Jupyter Notebook v6.5.6 and IPython 8.25.0 (more notes will
  - scikit-learn: 1.2.1 -> 1.5.0
  - statsmodels: 0.14.2 (new)
  - terminado: 0.17.1 -> 0.18.0
-  - tornado: 6.2 -> 6.4
+  - tornado: 6.2 -> 6.4.1


briehl · 2024-06-11T01:16:20Z

scripts/build_narrative_container.sh

-NAR_BASE="kbase/narrbase"
-NAR_BASE_VER="7.0"


Not anymore it doesn't!

ialarmedalien requested a review from briehl June 7, 2024 20:47

ialarmedalien self-assigned this Jun 7, 2024

ialarmedalien commented Jun 7, 2024

View reviewed changes

moving python-rqts file to the correct location

b5cf3ac

ialarmedalien commented Jun 9, 2024

View reviewed changes

briehl approved these changes Jun 11, 2024

View reviewed changes

ialarmedalien merged commit 59f5454 into develop Jun 11, 2024
6 of 10 checks passed

ialarmedalien deleted the random_updates branch June 11, 2024 12:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UIP-47: Misc tidying #3558

UIP-47: Misc tidying #3558

ialarmedalien commented Jun 7, 2024 •

edited

Loading

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

briehl Jun 11, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

briehl Jun 11, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

ialarmedalien Jun 7, 2024

sonarcloud bot commented Jun 7, 2024

codecov bot commented Jun 7, 2024

ialarmedalien Jun 9, 2024

briehl Jun 11, 2024

briehl left a comment

briehl Jun 11, 2024

briehl Jun 11, 2024

briehl Jun 11, 2024

		@@ -1,7 +1,9 @@
		"""Utils for implementing the KBase Narrative manager."""

		from typing import Any

		NAR_BASE="kbase/narrbase"
		NAR_BASE_VER="7.0"

		NAR_BASE="kbase/narrbase"
		NAR_BASE_VER="7.0"

UIP-47: Misc tidying #3558

UIP-47: Misc tidying #3558

Conversation

ialarmedalien commented Jun 7, 2024 • edited Loading

Description of PR purpose/changes

Jira Ticket / Issue

Testing Instructions

Dev Checklist:

Updating Version and Release Notes (if applicable)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jun 7, 2024

Quality Gate failed

codecov bot commented Jun 7, 2024

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briehl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ialarmedalien commented Jun 7, 2024 •

edited

Loading