Merge branch 'ml-evs/refcode-prefix' into v0.3.0

Author: ml-evs

pydatalab/docs/config.md

@@ -10,7 +10,18 @@
 3. Web app configuration, such as the URL of the relevant *datalab* API and branding (logo URLs, external homepage links).
     - These are typically provided as a `.env` file in the directory from which the webapp is built/served.
 
-## Configuring user registration/authentication
+## Mandatory settings
+
+There is only one mandatory setting when creating a deployment.
+This is the `IDENTIFIER_PREFIX`, which shall be prepended to every entry's refcode to enable global uniqueness of *datalab* entries.
+For now, the prefixes themselves are not checked for uniqueness across the fledling *datalab* federation, but will in the future.
+
+This prefix should be set to something relatively short (max 10 chars.) that describes your group or your deployment, e.g., the PI's surname, project ID or department.
+
+This can be set either via a config file, or as an environment variable (e.g., `PYDATALAB_IDENTIFIER_PREFIX='grey'`).
+Be warned, if the prefix changes between server launches, all entries will have to be migrated manually to the desired prefix, or maintained at the old prefix.
+
+## User registration & authentication
 
 *datalab* has three supported user registration/authentication
 mechanisms:
@@ -26,6 +37,7 @@ For GitHub, you must register a [GitHub OAuth
 application](https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/creating-an-oauth-app) for your instance, providing the client ID and secret in the `.env` for the API.
 Then, you can configure `GITHUB_ORG_ALLOW_LIST` with a list of string IDs of GitHub organizations that user's must be a public member of to register an account.
 If this value is set to `None`, then no accounts will be able to register, and if it is set to an empty list, then no restrictions will apply.
+You can find the relevant organization IDs using the GitHub API, for example at `https://api.github.com/orgs/<org_name>`.
 
 For ORCID integration, each *datalab* instance must currently register for the ORCID developer program and request new credentials.
 As such, this may be tricky to support for new instances.
@@ -36,7 +48,7 @@ additional configuration for the [SendGrid](https://sendgrid.com/) web API, i.e.
 There is currently no restrictions on which email addresses can sign up.
 This approach will soon also support using any configured SMTP server.
 
-## Configuring remote filesystems
+## Remote filesystems
 
 This package allows you to attach files from remote filesystems to samples and other entries.
 These filesystems can be configured in the config file with the `REMOTE_FILESYSTEMS` option.
@@ -47,7 +59,7 @@ Currently, there are two mechanisms for accessing remote files:
 1. You can mount the filesystem locally and provide the path in your datalab config file. For example, for Cambridge Chemistry users, you will have to (connect to the ChemNet VPN and) mount the Grey Group backup servers on your local machine, then define these folders in your config.
 2. Access over `ssh`: alternatively, you can set up passwordless `ssh` access to a machine (e.g., using `citadel` as a proxy jump), and paths on that remote machine can be configured as separate filesystems. The filesystem metadata will be synced periodically, and any files attached in `datalab` will be downloaded and stored locally on the `pydatalab` server (with the file being kept younger than 1 hour old on each access).
 
-## Server administration
+## General Server administration
 
 Currently most administration tasks must be handled directly inside the Python API container.
 Several helper routines are available as `invoke` tasks in `tasks.py` in the `pydatalab` root folder.

pydatalab/pydatalab/config.py

@@ -6,7 +6,15 @@
 from pathlib import Path
 from typing import Any, Dict, List, Optional, Type, Union
 
-from pydantic import AnyUrl, BaseModel, BaseSettings, Field, root_validator, validator
+from pydantic import (
+    AnyUrl,
+    BaseModel,
+    BaseSettings,
+    Field,
+    ValidationError,
+    root_validator,
+    validator,
+)
 
 from pydatalab.models import Person
 from pydatalab.models.utils import RandomAlphabeticalRefcodeFactory, RefCodeFactory
@@ -96,8 +104,8 @@ class ServerConfig(BaseSettings):
     )
 
     IDENTIFIER_PREFIX: str = Field(
-        "grey",
-        description="The prefix to use for identifiers in this deployment, e.g., `grey:AAAAAA`",
+        None,
+        description="The prefix to use for identifiers in this deployment, e.g., 'grey' in `grey:AAAAAA`",
     )
 
     REFCODE_GENERATOR: Type[RefCodeFactory] = Field(
@@ -150,6 +158,49 @@ def validate_cache_ages(cls, values):
             )
         return values
 
+    @validator("IDENTIFIER_PREFIX", pre=True, always=True)
+    def validate_identifier_prefix(cls, v, values):
+        """Make sure that the identifier prefix is set and is valid, raising clear error messages if not.
+
+        If in testing mode, then set the prefix to test too.
+
+        """
+
+        if values.get("TESTING"):
+            return "test"
+
+        if v is None:
+            import warnings
+
+            warning_msg = (
+                "You should configure an identifier prefix for this deployment. "
+                "You should attempt to make it unique to your deployment or group. "
+                "In the future these will be optionally globally validated versus all deployments for uniqueness. "
+                "For now the value of `test` will be used."
+            )
+
+            warnings.warn(warning_msg)
+            logging.warning(warning_msg)
+
+            return "test"
+
+        if len(v) > 12:
+            raise RuntimeError(
+                "Identifier prefix must be less than 12 characters long, received {v=}"
+            )
+
+        # test a trial refcode
+        from pydatalab.models.utils import Refcode
+
+        try:
+            Refcode(f"{v}:AAAAAA")
+        except ValidationError as exc:
+            raise RuntimeError(
+                f"Invalid identifier prefix: {v}. Validation with refcode `AAAAAA` returned error: {exc}"
+            )
+
+        return v
+
     class Config:
         env_prefix = "pydatalab_"
         extra = "allow"

pydatalab/pydatalab/models/items.py

@@ -47,11 +47,13 @@ class Item(Entry, HasOwner, HasRevisionControl, IsCollectable, HasBlocks, abc.AB
 
     @validator("refcode", pre=True, always=True)
     def refcode_validator(cls, v):
-        """Generate a refcode if not provided; check that the refcode has the correct prefix if provided."""
-
-        from pydatalab.config import CONFIG
-
-        if v and not v.startswith(f"{CONFIG.IDENTIFIER_PREFIX}:"):
-            raise ValueError(f"refcode missing prefix {CONFIG.IDENTIFIER_PREFIX!r}")
+        """Generate a refcode if not provided."""
+
+        if v:
+            prefix = None
+            id = None
+            prefix, id = v.split(":")
+            if prefix is None or id is None:
+                raise ValueError(f"refcode missing prefix or ID {id=}, {prefix=} from {v=}")
 
         return v

pydatalab/tests/routers/test_samples.py

@@ -161,7 +161,7 @@ def test_new_sample_with_relationships(client, complicated_sample):
     assert response.status_code == 201, response.json
     assert response.json["status"] == "success"
     new_refcode = response.json["sample_list_entry"]["refcode"]
-    assert new_refcode.startswith("grey:")
+    assert new_refcode.startswith("test:")
     assert response.json["sample_list_entry"]["item_id"] == complicated_sample.item_id
 
     response = client.get(
@@ -259,7 +259,7 @@ def test_saved_sample_has_new_relationships(client, default_sample_dict, complic
         f"/get-item-data/{default_sample_dict['item_id']}",
     )
     new_refcode = response.json["item_data"]["refcode"]
-    assert new_refcode.startswith("grey:")
+    assert new_refcode.startswith("test:")
 
     assert response.json
 

pydatalab/tests/test_config.py

@@ -1,5 +1,7 @@
 from pathlib import Path
 
+import pytest
+
 from pydatalab.config import ServerConfig
 from pydatalab.main import create_app
 
@@ -36,3 +38,13 @@ def test_config_override():
 
     assert CONFIG.REMOTE_FILESYSTEMS[0].hostname is None
     assert CONFIG.REMOTE_FILESYSTEMS[0].path == Path("/")
+
+
+def test_validators():
+    # check that prefix must be set
+    with pytest.warns():
+        _ = ServerConfig(IDENTIFIER_PREFIX=None)
+
+    # check bad prefix
+    with pytest.raises(RuntimeError):
+        _ = ServerConfig(IDENTIFIER_PREFIX="this prefix is way way too long")

webapp/src/components/FormattedRefcode.vue

@@ -34,11 +34,7 @@ export default {
       return "LightGrey";
     },
     shortenedName() {
-      if (this.refcode.includes(":")) {
-        return this.refcode.split(":")[1];
-      } else {
-        return this.refcode;
-      }
+      return this.refcode;
     },
   },
   methods: {