Merge pull request #621 from ably/fix/fix-tests-update-ci

chore: update CI / fix tests

Author: ttypic

.github/workflows/check.yml

@@ -20,20 +20,42 @@ jobs:
       matrix:
         python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', '3.12', '3.13']
     steps:
-    - uses: actions/checkout@v2
-      with:
-        submodules: 'recursive'
-    - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v2
-      with:
-        python-version: ${{ matrix.python-version }}
-    - name: Setup poetry
-      uses: abatilo/[email protected]
-      with:
-        poetry-version: 1.3.2
-    - name: Install dependencies
-      run: poetry install -E crypto
-    - name: Generate rest sync code and tests
-      run: poetry run unasync
-    - name: Test with pytest
-      run: poetry run pytest --verbose --tb=short
+      - uses: actions/checkout@v4
+        with:
+          submodules: 'recursive'
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v5
+        id: setup-python
+        with:
+          python-version: ${{ matrix.python-version }}
+
+      - name: Setup poetry
+        uses: abatilo/actions-poetry@v4
+        with:
+          poetry-version: '2.1.4'
+
+      - name: Setup a local virtual environment
+        run: |
+          poetry env use ${{ steps.setup-python.outputs.python-path }}
+          poetry run python --version
+          poetry config virtualenvs.create true --local
+          poetry config virtualenvs.in-project true --local
+
+      - uses: actions/cache@v4
+        name: Define a cache for the virtual environment based on the dependencies lock file
+        id: cache
+        with:
+          path: ./.venv
+          key: venv-${{ runner.os }}-${{ matrix.python-version }}-${{ hashFiles('poetry.lock') }}
+
+      - name: Ensure cache is healthy
+        if: steps.cache.outputs.cache-hit == 'true'
+        shell: bash
+        run: poetry run pip --version >/dev/null 2>&1 || (echo "Cache is broken, skip it" && rm -rf .venv)
+
+      - name: Install dependencies
+        run: poetry install -E crypto
+      - name: Generate rest sync code and tests
+        run: poetry run unasync
+      - name: Test with pytest
+        run: poetry run pytest --verbose --tb=short --reruns 3

.github/workflows/lint.yml

@@ -10,18 +10,40 @@ jobs:
   lint:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
-      with:
-        submodules: 'recursive'
-    - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v2
-      with:
-        python-version: '3.8'
-    - name: Setup poetry
-      uses: abatilo/[email protected]
-      with:
-        poetry-version: 1.3.2
-    - name: Install dependencies
-      run: poetry install -E crypto
-    - name: Lint with flake8
-      run: poetry run flake8
+      - uses: actions/checkout@v4
+        with:
+          submodules: 'recursive'
+      - name: Set up Python 3.9
+        uses: actions/setup-python@v5
+        id: setup-python
+        with:
+          python-version: '3.9'
+
+      - name: Setup poetry
+        uses: abatilo/actions-poetry@v4
+        with:
+          poetry-version: '2.1.4'
+
+      - name: Setup a local virtual environment
+        run: |
+          poetry env use ${{ steps.setup-python.outputs.python-path }}
+          poetry run python --version
+          poetry config virtualenvs.create true --local
+          poetry config virtualenvs.in-project true --local
+
+      - uses: actions/cache@v4
+        name: Define a cache for the virtual environment based on the dependencies lock file
+        id: cache
+        with:
+          path: ./.venv
+          key: venv-${{ runner.os }}-3.9-${{ hashFiles('poetry.lock') }}
+
+      - name: Ensure cache is healthy
+        if: steps.cache.outputs.cache-hit == 'true'
+        shell: bash
+        run: poetry run pip --version >/dev/null 2>&1 || (echo "Cache is broken, skip it." && rm -rf .venv)
+
+      - name: Install dependencies
+        run: poetry install
+      - name: Lint with flake8
+        run: poetry run flake8

ably/http/http.py

@@ -11,7 +11,7 @@
 from ably.http.httputils import HttpUtils
 from ably.transport.defaults import Defaults
 from ably.util.exceptions import AblyException
-from ably.util.helper import is_token_error
+from ably.util.helper import is_token_error, extract_url_params
 
 log = logging.getLogger(__name__)
 
@@ -198,11 +198,13 @@ def should_stop_retrying():
                                        self.preferred_port)
             url = urljoin(base_url, path)
 
+            (clean_url, url_params) = extract_url_params(url)
+
             request = self.__client.build_request(
                 method=method,
-                url=url,
+                url=clean_url,
                 content=body,
-                params=params,
+                params=dict(url_params, **params),
                 headers=all_headers,
                 timeout=timeout,
             )

ably/rest/auth.py

@@ -1,13 +1,16 @@
 from __future__ import annotations
+
 import base64
-from datetime import timedelta
 import logging
 import time
-from typing import Optional, TYPE_CHECKING, Union
 import uuid
+from datetime import timedelta
+from typing import Optional, TYPE_CHECKING, Union
+
 import httpx
 
 from ably.types.options import Options
+
 if TYPE_CHECKING:
     from ably.rest.rest import AblyRest
     from ably.realtime.realtime import AblyRealtime
@@ -16,14 +19,14 @@
 from ably.types.tokendetails import TokenDetails
 from ably.types.tokenrequest import TokenRequest
 from ably.util.exceptions import AblyAuthException, AblyException, IncompatibleClientIdException
+from ably.util.helper import extract_url_params
 
 __all__ = ["Auth"]
 
 log = logging.getLogger(__name__)
 
 
 class Auth:
-
     class Method:
         BASIC = "BASIC"
         TOKEN = "TOKEN"
@@ -271,8 +274,7 @@ async def create_token_request(self, token_params: Optional[dict | str] = None,
         if capability is not None:
             token_request['capability'] = str(Capability(capability))
 
-        token_request["client_id"] = (
-            token_params.get('client_id') or self.client_id)
+        token_request["client_id"] = token_params.get('client_id') or self.client_id
 
         # Note: There is no expectation that the client
         # specifies the nonce; this is done by the library
@@ -388,17 +390,27 @@ def _random_nonce(self):
 
     async def token_request_from_auth_url(self, method: str, url: str, token_params,
                                           headers, auth_params):
+        # Extract URL parameters using utility function
+        clean_url, url_params = extract_url_params(url)
+
         body = None
         params = None
         if method == 'GET':
             body = {}
-            params = dict(auth_params, **token_params)
+            # Merge URL params, auth_params, and token_params (later params override earlier ones)
+            # we do this because httpx version has inconsistency and some versions override query params
+            # that are specified in url string
+            params = {**url_params, **auth_params, **token_params}
         elif method == 'POST':
             if isinstance(auth_params, TokenDetails):
                 auth_params = auth_params.to_dict()
-            params = {}
+            # For POST, URL params go in query string, auth_params and token_params go in body
+            params = url_params
             body = dict(auth_params, **token_params)
 
+        # Use clean URL for the request
+        url = clean_url
+
         from ably.http.http import Response
         async with httpx.AsyncClient(http2=True) as client:
             resp = await client.request(method=method, url=url, headers=headers, params=params, data=body)
@@ -420,6 +432,6 @@ async def token_request_from_auth_url(self, method: str, url: str, token_params,
             token_request = response.text
         else:
             msg = 'auth_url responded with unacceptable content-type ' + content_type + \
-                ', should be either text/plain, application/jwt or application/json',
+                  ', should be either text/plain, application/jwt or application/json',
             raise AblyAuthException(msg, 401, 40170)
         return token_request

ably/util/helper.py

@@ -3,7 +3,8 @@
 import string
 import asyncio
 import time
-from typing import Callable
+from typing import Callable, Tuple, Dict
+from urllib.parse import urlparse, parse_qs
 
 
 def get_random_id():
@@ -25,6 +26,34 @@ def is_token_error(exception):
     return 40140 <= exception.code < 40150
 
 
+def extract_url_params(url: str) -> Tuple[str, Dict[str, str]]:
+    """
+    Extract URL parameters from a URL and return a clean URL and parameters dict.
+
+    Args:
+        url: The URL to parse
+
+    Returns:
+        Tuple of (clean_url_without_params, url_params_dict)
+    """
+    parsed_url = urlparse(url)
+    url_params = {}
+
+    if parsed_url.query:
+        # Convert query parameters to a flat dictionary
+        query_params = parse_qs(parsed_url.query)
+        for key, values in query_params.items():
+            # Take the last value if multiple values exist for the same key
+            url_params[key] = values[-1]
+
+    # Reconstruct clean URL without query parameters
+    clean_url = f"{parsed_url.scheme}://{parsed_url.netloc}{parsed_url.path}"
+    if parsed_url.fragment:
+        clean_url += f"#{parsed_url.fragment}"
+
+    return clean_url, url_params
+
+
 class Timer:
     def __init__(self, timeout: float, callback: Callable):
         self._timeout = timeout