feat(serverTransport): Add urlib3 retry policy to requests.Session clients used by ServerTransport (#461)

* Pre-commit on the public image

* change names

* first pass using urllib3 retry policy

* add some basic unit test

* correct the doc string

.github/workflows/pr.yml

@@ -9,8 +9,55 @@ on:
       - "main"
 
 jobs:
-  test:
-    name: test
+  test-internal: # Run integration tests against the internal server image
+    name: Test (internal)
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version:
+          - "3.10"
+          - "3.11"
+          - "3.12"
+          - "3.13"
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install uv and set the python version
+        uses: astral-sh/setup-uv@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+          enable-cache: true
+          cache-dependency-glob: "uv.lock"
+
+      - name: Install the project
+        run: uv sync --all-extras --dev
+
+      - name: Login to GitHub Container Registry
+        uses: docker/login-action@v3
+        with:
+          registry: "ghcr.io"
+          username: ${{ github.actor }}
+          password: ${{ github.token }}
+
+      - name: Run Speckle Server
+        run: docker compose --file docker-compose-internal.yml up --detach --wait
+
+      - name: Run tests
+        run: uv run pytest --cov --cov-report xml:reports/coverage.xml --junitxml=reports/test-results.xml
+
+      - uses: codecov/codecov-action@v5
+        if: matrix.python-version == 3.12
+        with:
+          fail_ci_if_error: true # optional (default = false)
+          files: ./reports/test-results.xml # optional
+          token: ${{ secrets.CODECOV_TOKEN }}
+
+      - name: Minimize uv cache
+        run: uv cache prune --ci
+
+  test-public: # Run integration tests against the public server image
+    name: Test (public)
     runs-on: ubuntu-latest
     strategy:
       matrix:
@@ -42,17 +89,10 @@ jobs:
         run: uv run pre-commit run --all-files
 
       - name: Run Speckle Server
-        run: docker compose up --detach --wait
+        run: docker compose --file docker-compose.yml up --detach --wait
 
       - name: Run tests
         run: uv run pytest --cov --cov-report xml:reports/coverage.xml --junitxml=reports/test-results.xml
 
-      - uses: codecov/codecov-action@v5
-        if: matrix.python-version == 3.12
-        with:
-          fail_ci_if_error: true # optional (default = false)
-          files: ./reports/test-results.xml # optional
-          token: ${{ secrets.CODECOV_TOKEN }}
-
       - name: Minimize uv cache
         run: uv cache prune --ci

docker-compose-internal.yml

@@ -0,0 +1,119 @@
+name: "speckle-server"
+
+services:
+  ####
+  # Speckle Server dependencies
+  #######
+  postgres:
+    image: "postgres:16.4-alpine3.20@sha256:d898b0b78a2627cb4ee63464a14efc9d296884f1b28c841b0ab7d7c42f1fffdf"
+    restart: always
+    environment:
+      POSTGRES_DB: speckle
+      POSTGRES_USER: speckle
+      POSTGRES_PASSWORD: speckle
+    volumes:
+      - ./.volumes/postgres-data:/var/lib/postgresql/data/
+    healthcheck:
+      # the -U user has to match the POSTGRES_USER value
+      test: ["CMD-SHELL", "pg_isready -U speckle"]
+      interval: 5s
+      timeout: 5s
+      retries: 30
+
+  redis:
+    image: "valkey/valkey:8.1-alpine@sha256:0d27f0bca0249f61d060029a6aaf2e16b2c417d68d02a508e1dfb763fa2948b4"
+    restart: always
+    volumes:
+      - ./.volumes/redis-data:/data
+    healthcheck:
+      test: ["CMD", "redis-cli", "--raw", "incr", "ping"]
+      interval: 5s
+      timeout: 5s
+      retries: 30
+
+  minio:
+    image: "minio/minio:RELEASE.2023-10-25T06-33-25Z"
+    command: server /data --console-address ":9001"
+    restart: always
+    volumes:
+      - ./.volumes/minio-data:/data
+    ports:
+      - '127.0.0.1:9000:9000'
+      - '127.0.0.1:9001:9001'
+    healthcheck:
+      test:
+        [
+          "CMD-SHELL",
+          "curl -s -o /dev/null http://127.0.0.1:9000/minio/index.html",
+        ]
+      interval: 5s
+      timeout: 30s
+      retries: 30
+      start_period: 10s
+
+  speckle-server:
+    image: ghcr.io/specklesystems/speckle-server:latest
+    restart: always
+    healthcheck:
+      test:
+        - CMD
+        - /nodejs/bin/node
+        - -e
+        - "try { require('node:http').request({headers: {'Content-Type': 'application/json'}, port:3000, hostname:'127.0.0.1', path:'/readiness', method: 'GET', timeout: 2000 }, (res) => { body = ''; res.on('data', (chunk) => {body += chunk;}); res.on('end', () => {process.exit(Number(res.statusCode != 200 || body.toLowerCase().includes('error')));}); }).end(); } catch { process.exit(1); }"
+      interval: 10s
+      timeout: 10s
+      retries: 3
+      start_period: 90s
+    ports:
+      - "0.0.0.0:3000:3000"
+    depends_on:
+      postgres:
+        condition: service_healthy
+      redis:
+        condition: service_healthy
+      minio:
+        condition: service_healthy
+    environment:
+      # TODO: Change this to the URL of the speckle server, as accessed from the network
+      CANONICAL_URL: "http://127.0.0.1:8080"
+      SPECKLE_AUTOMATE_URL: "http://127.0.0.1:3030"
+      FRONTEND_ORIGIN: "http://127.0.0.1:8081"
+
+      # TODO: Change thvolumes:
+      REDIS_URL: "redis://redis"
+
+      S3_ENDPOINT: "http://minio:9000"
+      S3_PUBLIC_ENDPOINT: "http://127.0.0.1:9000"
+      S3_ACCESS_KEY: "minioadmin"
+      S3_SECRET_KEY: "minioadmin"
+      S3_BUCKET: "speckle-server"
+      S3_CREATE_BUCKET: "true"
+
+      FILE_SIZE_LIMIT_MB: 100
+      MAX_PROJECT_MODELS_PER_PAGE: 500
+
+      # TODO: Change this to a unique secret for this server
+      SESSION_SECRET: "TODO:ReplaceWithLongString"
+
+      STRATEGY_LOCAL: "true"
+      DEBUG: "speckle:*"
+
+      POSTGRES_URL: "postgres"
+      POSTGRES_USER: "speckle"
+      POSTGRES_PASSWORD: "speckle"
+      POSTGRES_DB: "speckle"
+      ENABLE_MP: "false"
+
+      LOG_PRETTY: "true"
+
+      FF_NEXT_GEN_FILE_IMPORTER_ENABLED: "true"
+      FF_LARGE_FILE_IMPORTS_ENABLED: "true"
+
+networks:
+  default:
+    name: speckle-server
+
+volumes:
+  postgres-data:
+  redis-data:
+  minio-data:

docker-compose.yml

@@ -1,4 +1,3 @@
-version: "3.9"
 name: "speckle-server"
 
 services:
@@ -22,7 +21,7 @@ services:
       retries: 30
 
   redis:
-    image: "redis:6.0-alpine"
+    image: "valkey/valkey:8.1-alpine@sha256:0d27f0bca0249f61d060029a6aaf2e16b2c417d68d02a508e1dfb763fa2948b4"
     restart: always
     volumes:
       - ./.volumes/redis-data:/data
@@ -38,6 +37,9 @@ services:
     restart: always
     volumes:
       - ./.volumes/minio-data:/data
+    ports:
+      - '127.0.0.1:9000:9000'
+      - '127.0.0.1:9001:9001'
     healthcheck:
       test:
         [
@@ -48,8 +50,6 @@ services:
       timeout: 30s
       retries: 30
       start_period: 10s
-    ports:
-      - "0.0.0.0:9000:9000"
 
   speckle-server:
     image: speckle/speckle-server:latest
@@ -104,9 +104,10 @@ services:
       POSTGRES_DB: "speckle"
       ENABLE_MP: "false"
 
+      LOG_PRETTY: "true"
+
       FF_NEXT_GEN_FILE_IMPORTER_ENABLED: "true"
       FF_LARGE_FILE_IMPORTS_ENABLED: "true"
-      FF_BACKGROUND_JOBS_ENABLED: "true"
 
 networks:
   default:

pyproject.toml

@@ -11,7 +11,7 @@ dependencies = [
     "appdirs>=1.4.4",
     "attrs>=24.3.0",
     "deprecated>=1.2.15",
-    "gql[requests,websockets]>=3.5.0",
+    "gql[requests,websockets]>=3.5.0,<4.0.0",
     "httpx>=0.28.1",
     "pydantic>=2.10.5",
     "pydantic-settings>=2.7.1",

src/speckleifc/importer.py

@@ -54,9 +54,10 @@ class ImportJob:
             )
 
         children = self._convert_children(step_element)
-        display_value = self.cached_display_values.get(step_element.id(), [])
+        id = step_element.id()
+        display_value = self.cached_display_values.get(id, [])
 
-        if display_value is not None:
+        if display_value:
             self.geometries_used += 1
 
         # Extract current storey name from DataObject if available

src/speckleifc/property_extraction.py

@@ -1,3 +1,4 @@
+import math
 from typing import Any, Tuple
 
 from ifcopenshell.entity_instance import entity_instance
@@ -134,6 +135,10 @@ def _get_quantities(
             value = getattr(quantity, quantity.attribute_name(3))
             unit_info = _get_unit_info(element, quantity)
 
+            # Server does not consider `NaN` valid json
+            if math.isnan(value):
+                value = None
+
             if unit_info:
                 # Create structured quantity object with units
                 results[quantity_name] = {

src/specklepy/core/api/wrapper.py

@@ -1,8 +1,6 @@
 from urllib.parse import quote, unquote, urlparse
 from warnings import warn
 
-from gql import gql
-
 from specklepy.core.api.client import SpeckleClient
 from specklepy.core.api.credentials import (
     Account,
@@ -139,27 +137,11 @@ class StreamWrapper:
 
         if use_fe2 is True and self.branch_name is not None:
             self.model_id = self.branch_name
-            # get branch name
-            query = gql(
-                """
-                query Project($project_id: String!, $model_id: String!) {
-                    project(id: $project_id) {
-                        id
-                        model(id: $model_id) {
-                            name
-                        }
-                    }
-                }
-            """
-            )
-            self._client = self.get_client()
-            params = {"project_id": self.stream_id, "model_id": self.model_id}
-            project = self._client.httpclient.execute(query, params)
 
-            try:
-                self.branch_name = project["project"]["model"]["name"]
-            except KeyError as ke:
-                raise SpeckleException("Project model name is not found", ke) from ke
+            self._client = self.get_client()
+            model = self._client.model.get(self.model_id, self.stream_id)
+
+            self.branch_name = model.name
 
         if not self.stream_id:
             raise SpeckleException(
@@ -175,6 +157,10 @@ class StreamWrapper:
         """
         Gets an account object for this server from the local accounts db
         (added via Speckle Manager or a json file)
+
+        WARNING: this function will return ANY account for the server,
+          just because you pass a token in doesn't guarantee it will be used.
+          This whole class could do with a re-design...
         """
         if self._account and self._account.token:
             return self._account

src/specklepy/transports/server/batch_sender.py

@@ -7,6 +7,7 @@ import threading
 import requests
 
 from specklepy.logging.exceptions import SpeckleException
+from specklepy.transports.server.retry_policy import setup_session
 
 LOG = logging.getLogger(__name__)
 
@@ -72,10 +73,7 @@ class BatchSender:
 
     def _sending_thread_main(self):
         try:
-            session = requests.Session()
-            session.headers.update(
-                {"Authorization": f"Bearer {self._token}", "Accept": "text/plain"}
-            )
+            session = setup_session(self._token)
 
             while True:
                 batch = self._batches.get()

src/specklepy/transports/server/retry_policy.py

@@ -0,0 +1,46 @@
+import requests
+from requests.adapters import HTTPAdapter
+from urllib3 import Retry
+
+
+def setup_session(auth_token: str | None) -> requests.Session:
+    """
+    Sets up a requests.Session with a basic retry policy
+    to retry on all the usual retryable status codes, with a back off policy:
+        1st: 0ms,
+        2nd: 500ms,
+        3rd: 1500ms.
+
+    Also sets "Accept": "text/plain" header (because this is what ServerTransport needs)
+    and (if a auth_token is provided) the Authorization header
+    """
+
+    session = requests.Session()
+    retry_policy = Retry(
+        total=3,
+        read=3,
+        connect=3,
+        backoff_factor=0.5,
+        status_forcelist=(500, 502, 503, 504, 408, 429),
+        allowed_methods=["HEAD", "GET", "OPTIONS", "POST", "PUT", "DELETE"],
+        raise_on_status=False,
+    )
+
+    adapter = HTTPAdapter(max_retries=retry_policy)
+    session.mount("https://", adapter)
+    session.mount("http://", adapter)
+
+    session.headers.update(
+        {
+            "Accept": "text/plain",
+        }
+    )
+
+    if auth_token is not None:
+        session.headers.update(
+            {
+                "Authorization": f"Bearer {auth_token}",
+            }
+        )
+
+    return session

src/specklepy/transports/server/server.py

@@ -2,12 +2,11 @@ import json
 from typing import Dict, List, Optional
 from warnings import warn
 
-import requests
-
 from specklepy.core.api.client import SpeckleClient
 from specklepy.core.api.credentials import Account, get_account_from_token
 from specklepy.logging.exceptions import SpeckleException, SpeckleWarning
 from specklepy.transports.abstract_transport import AbstractTransport
+from specklepy.transports.server.retry_policy import setup_session
 
 from .batch_sender import BatchSender
 
@@ -92,23 +91,13 @@ class ServerTransport(AbstractTransport):
         self.stream_id = stream_id
         self.url = url
 
-        self.session = requests.Session()
-
-        self.session.headers.update(
-            {
-                "Accept": "text/plain",
-            }
-        )
-
         if self.account is not None:
             self._batch_sender = BatchSender(
                 self.url, self.stream_id, self.account.token, max_batch_size_mb=1
             )
-            self.session.headers.update(
-                {
-                    "Authorization": f"Bearer {self.account.token}",
-                }
-            )
+        self.session = setup_session(
+            self.account.token if self.account is not None else None
+        )
 
     @property
     def name(self) -> str:

tests/unit/test_setup_session.py

@@ -0,0 +1,20 @@
+import requests
+
+from specklepy.transports.server.retry_policy import setup_session
+
+
+def test_session_headers_without_auth():
+    """Check that Accept header is set and Authorization is not."""
+    session = setup_session(None)
+    assert isinstance(session, requests.Session)
+    assert session.headers["Accept"] == "text/plain"
+    assert "Authorization" not in session.headers
+
+
+def test_session_headers_with_auth():
+    """Check that Authorization header is properly added."""
+    token = "abc123"
+    session = setup_session(token)
+    assert isinstance(session, requests.Session)
+    assert session.headers["Authorization"] == f"Bearer {token}"
+    assert session.headers["Accept"] == "text/plain"