Address feedback from spec review: ooni/spec#292

* Add support for expiration date
* Compute is_expired from expiration date at eval time
* Add support for color
* Drop creator_account_id from return value
* Drop v string
* Small bugfixing
* Add alembic db migration
* Fix tests and some bugs related to expiration time
* Add more tests for expiration
* Add more tests for OONI Run v2
* Reach 100% code coverage
* Bump minor version number
* Interpolate the OONI_PG_PASSWORD
* Convert OONI Run link ID column to string
* Handle the _intl fields being set to None
* Bump API version to 0.4.1
* Add tests for filtering by only_latest
* Add click to dev deps
* Fix types of oonirun_link_id
* Add a smoketest for oonirun v2
* Add a smoketest stage before pushing the docker tags
* Rename test_new_api to test_legacy_ooniapi
* Move mypy tests into test_legacy_ooniapi
* Run alembic migration before starting pg host
* bump dataapi version

.github/workflows/build_dataapi.yml

@@ -14,8 +14,28 @@ env:
   IMAGE_NAME: ooni/dataapi
 
 jobs:
+  test:
+    uses: ./.github/workflows/test_dataapi.yml
+
   build_and_push:
+    name: Build and push
+    needs: [test]
     runs-on: ubuntu-latest
+    services:
+      postgres:
+        image: postgres
+        env:
+          POSTGRES_USER: oonipg
+          POSTGRES_PASSWORD: oonipg
+          POSTGRES_DB: oonipg
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 5432:5432
+
     steps:
       - name: Checkout Repository
         uses: actions/checkout@v2
@@ -37,6 +57,7 @@ jobs:
           echo "version_number=$VERSION_NUMBER" >> "$GITHUB_OUTPUT"
 
       - name: Build and Push Docker Image
+        id: dockerbuild
         env:
           DOCKERFILE_PATH: ${{ env.oonidataapi_dir }}
         run: |
@@ -45,19 +66,67 @@ jobs:
           TAG_BUILD_LABEL=$IMAGE_NAME:${{ steps.version.outputs.build_label }}
           TAG_VERSION=$IMAGE_NAME:v${{ steps.version.outputs.version_number }}
 
+          echo "tag_latest=$TAG_LATEST" >> $GITHUB_OUTPUT
+          echo "tag_environment=$TAG_ENVIRONMENT" >> $GITHUB_OUTPUT
+          echo "tag_build_label=$TAG_BUILD_LABEL" >> $GITHUB_OUTPUT
+          echo "tag_version=$TAG_VERSION" >> $GITHUB_OUTPUT
+
           # Build Docker image with multiple tags
           docker build --build-arg BUILD_LABEL=${{ steps.version.outputs.build_label }} \
                        -t $TAG_BUILD_LABEL \
                        -t $TAG_ENVIRONMENT \
                        -t $TAG_LATEST \
                        -t $TAG_VERSION \
                        $DOCKERFILE_PATH
+      # Setup python
+      - name: Set up Python 3.11
+        uses: actions/setup-python@v4
+        with:
+          python-version: 3.11
 
+      - name: Install poetry
+        run: |
+          curl -fsS https://install.python-poetry.org | python - --preview -y
+
+      - name: Add poetry to PATH
+        run: echo "$HOME/.local/bin" >> $GITHUB_PATH
+
+      - name: Set up poetry cache
+        uses: actions/cache@v3
+        with:
+          path: "$HOME/.cache/pypoetry/virtualenvs"
+          key: venv-${{ runner.os }}-${{ hashFiles('**/api/fastapi/poetry.lock') }}
+
+      - name: Install dependencies
+        run: poetry install
+        working-directory: ./api/fastapi/
+
+      # Configure database and docker
+      - name: Run alembic migrations
+        env:
+          OONI_PG_PASSWORD: oonipg
+          OONI_PG_HOST: localhost
+        run: poetry run alembic upgrade head
+        working-directory: ./api/fastapi/oonidataapi/
+
+      - name: Start Docker container with PostgreSQL
+        run: |
+          docker run -d --name oonidataapi -p 8000:80 \
+            -e POSTGRESQL_URL="postgresql://oonipg:oonipg@localhost/oonipg" \
+            ${{ steps.dockerbuild.outputs.tag_version }}
+
+      # Run smoke test
+      #- name: Run smoketest against the built docker image
+      #  run: poetry run python oonidataapi/tests/run_smoketest.py --backend-base-url=http://localhost:8000/
+      #  working-directory: ./api/fastapi/
+
+      - name: Push docker tags
+        run: |
           # Push all tags
-          docker push $TAG_BUILD_LABEL
-          docker push $TAG_ENVIRONMENT
-          docker push $TAG_LATEST
-          docker push $TAG_VERSION
+          docker push ${{ steps.dockerbuild.outputs.tag_latest }}
+          docker push ${{ steps.dockerbuild.outputs.tag_environment }}
+          docker push ${{ steps.dockerbuild.outputs.tag_build_label }}
+          docker push ${{ steps.dockerbuild.outputs.tag_version }}
 
       #- name: Checkout ooni/devops
       #  uses: actions/checkout@v2

.github/workflows/test_dataapi.yml

@@ -6,6 +6,7 @@ on:
   pull_request:
     branches:
       - "*"
+  workflow_call:
 jobs:
   run_tests:
     runs-on: ubuntu-latest

.github/workflows/test_new_api.yml → .github/workflows/test_legacy_ooniapi.py

@@ -1,4 +1,4 @@
-name: Test API
+name: Test Legacy API
 on:
   pull_request:
   workflow_dispatch:
@@ -9,6 +9,29 @@
         default: false
 
 jobs:
+  mypy:
+    runs-on: ubuntu-latest
+    container: debian:11
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Setup APT
+        run: |
+          apt-get update
+          apt-get install --no-install-recommends -y ca-certificates gnupg
+          echo "deb http://deb-ci.ooni.org unstable main" >> /etc/apt/sources.list
+          apt-key adv --verbose --keyserver hkp://keyserver.ubuntu.com --recv-keys "B5A08F01796E7F521861B449372D1FF271F2DD50"
+
+      - name: Install dependencies
+        run: |
+          apt-get update
+          apt-get install --no-install-recommends -qy mypy
+
+      - name: Run tests
+        # see the mypy.ini file
+        run: cd api && mypy **/*.py
+
   integration_test:
     runs-on: ubuntu-latest
     steps:

api/fastapi/oonidataapi/alembic.ini

@@ -60,7 +60,7 @@ version_path_separator = os  # Use os.pathsep. Default configuration used for ne
 # are written from script.py.mako
 # output_encoding = utf-8
 
-sqlalchemy.url = postgresql://oonipg@postgres.tier0.prod.ooni.nu/oonipg
+sqlalchemy.url = postgresql://oonipg:%(OONI_PG_PASSWORD)s@%(OONI_PG_HOST)s/oonipg
 
 
 [post_write_hooks]

api/fastapi/oonidataapi/alembic/Readme.md

@@ -11,7 +11,7 @@ poetry run alembic revision -m "name of the revision"
 2. Edit the newly created python file and fill out the `upgrade()` and `downgrade()` function with the relevant code bits
 3. You can now run the migration like so:
 ```
-poetry run alembic upgrade head
+OONI_PG_PASSWORD=XXXX poetry run alembic upgrade head
 ```
 
 

api/fastapi/oonidataapi/alembic/env.py

@@ -1,3 +1,5 @@
+import os
+
 from logging.config import fileConfig
 
 from sqlalchemy import engine_from_config
@@ -19,8 +21,17 @@
 # from myapp import mymodel
 # target_metadata = mymodel.Base.metadata
 from oonidataapi import models
+
 target_metadata = models.Base.metadata
 
+section = config.config_ini_section
+config.set_section_option(
+    section, "OONI_PG_PASSWORD", os.environ.get("OONI_PG_PASSWORD", "")
+)
+config.set_section_option(
+    section, "OONI_PG_HOST", os.environ.get("OONI_PG_HOST", "postgres.tier0.prod.ooni.nu")
+)
+
 # other values from the config, defined by the needs of env.py,
 # can be acquired:
 # my_important_option = config.get_main_option("my_important_option")
@@ -65,9 +76,7 @@ def run_migrations_online() -> None:
     )
 
     with connectable.connect() as connection:
-        context.configure(
-            connection=connection, target_metadata=target_metadata
-        )
+        context.configure(connection=connection, target_metadata=target_metadata)
 
         with context.begin_transaction():
             context.run_migrations()

api/fastapi/oonidataapi/alembic/versions/7d5841cb9549_make_oonirun_link_id_a_string.py

@@ -0,0 +1,37 @@
+"""make oonirun link id a string
+
+Revision ID: 7d5841cb9549
+Revises: 836b3451a168
+Create Date: 2024-02-28 15:41:53.811746
+
+"""
+
+from typing import Sequence, Union
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision: str = "7d5841cb9549"
+down_revision: Union[str, None] = "836b3451a168"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    op.execute(
+        """
+        ALTER TABLE oonirun
+        ALTER COLUMN oonirun_link_id TYPE TEXT USING oonirun_link_id::TEXT
+    """
+    )
+
+
+def downgrade() -> None:
+    op.execute(
+        """
+        ALTER TABLE oonirun
+        ALTER COLUMN oonirun TYPE INTEGER USING oonirun::INTEGER
+    """
+    )

api/fastapi/oonidataapi/alembic/versions/836b3451a168_add_expiration_date_color_columns_drop_.py

@@ -0,0 +1,32 @@
+"""Add expiration_date, color columns. Drop is_archived column.
+
+Revision ID: 836b3451a168
+Revises: f96cf47f2791
+Create Date: 2024-02-27 09:44:26.833238
+
+"""
+
+from typing import Sequence, Union
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision: str = "836b3451a168"
+down_revision: Union[str, None] = "f96cf47f2791"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    op.add_column(
+        "oonirun", sa.Column("expiration_date", sa.DateTime(), nullable=False)
+    )
+    op.add_column("oonirun", sa.Column("color", sa.String(), nullable=True))
+    op.drop_column("oonirun", "is_archived")
+
+
+def downgrade() -> None:
+    op.drop_column("oonirun", "expiration_date")
+    op.drop_column("oonirun", "color")

api/fastapi/oonidataapi/models.py

@@ -1,3 +1,4 @@
+from datetime import timezone
 from sqlalchemy import Boolean, Column, Integer, String, DateTime, JSON
 
 from .postgresql import Base
@@ -6,12 +7,26 @@
 class OONIRunLink(Base):
     __tablename__ = "oonirun"
 
-    oonirun_link_id = Column(Integer, primary_key=True)
+    oonirun_link_id = Column(String, primary_key=True)
     revision = Column(Integer, default=1, primary_key=True)
     date_updated = Column(DateTime)
     date_created = Column(DateTime)
     creator_account_id = Column(String)
 
+    expiration_date = Column(DateTime, nullable=False)
+
+    # Timezones are kind of tricky. We assume everything is always in UTC,
+    # but python, rightfully complains, if that encoding is not specified in
+    # the object itself since more modern versions of python.
+    # To avoid making this a DB specific change, we don't introduce the
+    # TIMESTAMP column which would allow us to retrieve timezone native
+    # objects, but instead do casting to the timezone native equivalent in
+    # the code.
+    # See: https://stackoverflow.com/questions/414952/sqlalchemy-datetime-timezone
+    @property
+    def expiration_date_dt_native(self):
+        return self.expiration_date.replace(tzinfo=timezone.utc)
+
     name = Column(String)
     name_intl = Column(JSON, nullable=True)
     short_description = Column(String)
@@ -20,5 +35,5 @@ class OONIRunLink(Base):
     description_intl = Column(JSON, nullable=True)
     author = Column(String)
     icon = Column(String)
+    color = Column(String)
     nettests = Column(JSON)
-    is_archived = Column(Boolean, default=False)