API tokens authentication in SDK (APPS-14746) by jszymanski-rtbh · Pull Request #191 · rtbhouse-apps/rtbhouse-python-sdk

jszymanski-rtbh · 2026-02-02T09:20:55Z

Added ApiTokenAuth for authenticating with a fixed API token.
Introduced DynamicApiTokenAuth / AsyncDynamicApiTokenAuth for provider-based authentication with per-request token resolution. Tokens are persisted in storage and automatically rotated when entering the rotation window.
Implemented token endpoints clients (sync + async): heartbeat() and rotate().
Added token managers (sync + async) providing:
- rotation/expiration handling,
- configure() and configure_from_env() helpers.
Added token storage backends:
- InMemoryApiTokenStorage and JsonFileApiTokenStorage,
- async variants for both.

jszymanski-rtbh · 2026-02-02T09:24:57Z

Testy oraz więcej przykładów używania w changelog, dodam po wstępnym zaakceptowaniu architektury rozwiązania.

- Introduced `DynamicApiTokenAuth` / `AsyncDynamicApiTokenAuth` for provider-based authentication with per-request token resolution. Tokens are persisted in storage and automatically rotated when entering the rotation window. - Implemented token endpoints clients (sync + async): `heartbeat()` and `rotate()`. - Added token managers (sync + async) providing: - rotation/expiration handling, - `configure()` and `configure_from_env()` helpers. - Added token storage backends: - `InMemoryApiTokenStorage` and `JsonFileApiTokenStorage`, - async variants for both.

jszymanski-rtbh · 2026-02-02T09:26:31Z

rtbhouse_sdk/api_tokens/storages/json_file.py

+    def _atomic_write_text(self, text: str) -> None:
+        descriptor, temp_path = tempfile.mkstemp(
+            prefix=self._path.name + ".",
+            dir=self._path.parent,
+        )
+        try:
+            with os.fdopen(
+                descriptor,
+                "w",
+                encoding="utf-8",
+            ) as file:
+                file.write(text)
+                file.flush()
+                os.fsync(file.fileno())
+
+            try:
+                os.chmod(temp_path, 0o600)
+            except OSError:
+                pass
+
+            os.replace(temp_path, self._path)
+
+        finally:
+            try:
+                os.remove(temp_path)
+            except FileNotFoundError:
+                pass
+            except OSError:
+                pass


Tutaj sugerowałem się tymi źródłami:

https://stackoverflow.com/questions/2333872/how-to-make-file-creation-an-atomic-operation

https://python.plainenglish.io/simple-safe-atomic-writes-in-python3-44b98830a013

rtbhouse_sdk/api_tokens/storages/json_file.py

rtbhouse_sdk/api_tokens/managers.py

…n bool

peku33

Wygląda nieźle, zostawiłem trochę komentarzy 💪

Na pewno warto unormować sposób inicjalizacji i deklaracji zmiennych w klasach:

czasami wołasz super().__init__(), a czasami nie. Proponuję wołać zawsze, nawet jeśli dziedziczymy tylko po object. Dzięki temu jeśli przypadkiem pojawi nam się kiedyś klasa bazowa z jakimiś zmiennymi - nie pominiemy przypadkiem wywołania, bo linter nam to złapie
proponuję generalnie zadeklarować składowe w klasie, żeby patrząc na samą klasę było mniej więcej widać z czego się składa.

rtbhouse_sdk/api_tokens/api.py

rtbhouse_sdk/api_tokens/managers.py

rtbhouse_sdk/api_tokens/storages/json_file.py

peku33 · 2026-02-10T12:48:49Z

rtbhouse_sdk/api_tokens/storages/json_file.py

+        except FileNotFoundError:
+            pass
+
+    def _atomic_write_text(self, text: str) -> None:


Wydaje mi się, że to nie rozwiązuje nam do końca problemu. O ile lock na managerze jest sensowny, to chyba nie chroni nas nijak przed różnymi wątkami/procesami aktualizaującymi ten sam plik. To chyba nie jest jakiś straszny problem, ale warto mieć go z tyłu głowy.

Swoją drogą - może powinniśmy w ogóle mieć tu metodę typu update_with() która jest contextmanagerem zapewniającym lock na źródle?

Chyba kwestię tego locka na pliku możemy odpuścić. Manager uruchamia zapis do pliku (save) tylko po udanej rotacji. Nasze API przepuści tylko jeden request z rotacją.

Tutaj tym atomic_write chciałem poradzić sobie z sytuacją jakiegoś połowicznego odczytu/zapisu który skończyłby się błędem

rtbhouse_sdk/client.py

peku33 · 2026-03-12T14:08:22Z

pyproject.toml

+cachetools = "^7.0.3"
+types-cachetools = "^6.2.0.20251022"


Prośba o posortowanie importów, samo types-cachetools może iść do grupki .dev

peku33 · 2026-03-12T14:09:12Z

CHANGELOG.md

@@ -1,3 +1,29 @@
+# v16.0.0


Czy mamy jakiś breaking change? Jeśli tak - warto go tu opisać wraz z instrukcją migracji. Jeśli nie - może wystarczy zrobić z tego wersję 15.1.0?

peku33 · 2026-03-12T14:10:47Z

rtbhouse_sdk/__main__.py

Czy potrzebujemy tego pliku? Może jednak wystarczy __main__.py w api_tokens wywołany bezpośrednio?

peku33 · 2026-03-12T14:21:01Z

rtbhouse_sdk/client.py

+        try:
+            resp_json = response.json()
+            return resp_json["data"]
+        except (ValueError, KeyError) as exc:
+            raise ApiException("Invalid response format") from exc


To nam się w sumie powtarza pomiędzy miejscami, więc można:

zamknąć to w _validate_response

zamiast _get i _post mieć `_request(method, path, params, data)

peku33 · 2026-03-12T14:21:13Z

rtbhouse_sdk/client.py

        except (ValueError, KeyError) as exc:
            raise ApiException("Invalid response format") from exc

+    def _post(self, path: str, data: dict[str, Any] | None = None, params: dict[str, Any] | None = None) -> Any:


Na logikę params powinno iść po path.

peku33 · 2026-03-12T14:21:32Z

rtbhouse_sdk/client.py

+    def _post_dict(
+        self, path: str, data: dict[str, Any] | None = None, params: dict[str, Any] | None = None


analogicznie tu i w drugim kliencie

peku33 · 2026-03-12T14:35:38Z

rtbhouse_sdk/client.py

+    async def get_current_api_token(self) -> schema.ApiToken:
+        data = await self._get_dict("/tokens/current")
+        return schema.ApiToken(**data)
+
+    async def rotate_current_api_token(self) -> schema.RotatedApiToken:
+        data = await self._post_dict("/tokens/current/rotate")
+        return schema.RotatedApiToken(**data)


Te modele to nie jest bardziej jakiś "ApiTokenDetails" vs "ApiTokenRotateResult"?

peku33 · 2026-03-12T14:36:14Z

rtbhouse_sdk/client.py

+        return schema.RotatedApiToken(**data)
+
+
+class _HttpxApiTokenAuth(httpx.Auth):


Prośba o podeklarowanie składowych w klasach i wołanie super.init()

peku33 · 2026-03-12T15:40:30Z

rtbhouse_sdk/api_tokens/cli.py

Ponieważ dość mocno nam się rozmyła struktura w tym module, proponuję zrealizować go przy użyciu clicka (opcjonalnie typera, ale ten jest zdaje się dużo cięższy i m awięcej zależności).

Mamy obecnie coś takiego:

handler 1

handler 2

handler 3

builder

argsy 1

argsy 2

argsy 3

wołanie buildera

Helpery przerzućmy na dół.

Przekazywaniem tokena przez argsy cli / env proponuję zastąpić przekazywaniem tokena przez stdin/prompt, systemowy shell obsłuży natywnie dwa pozostałe przypadki. Proponuję spojrzeć co click/typer oferuje na takie okazje, być może po swojemu obsługuje stdin.

Zamiast zaślepiać pustym stringiem - proponuję wziąć pod uwagę stałą długość tokenu (albo przynajmniej założyć że nie może być pusty). Mamy też None.

jszymanski-rtbh requested a review from a team as a code owner February 2, 2026 09:20

jszymanski-rtbh removed the request for review from a team February 2, 2026 09:21

jszymanski-rtbh force-pushed the APPS-14746-api-tokens-in-sdk branch from 562ac52 to 366d5b3 Compare February 2, 2026 09:31

jszymanski-rtbh commented Feb 2, 2026

View reviewed changes

jszymanski-rtbh requested a review from bgoral-rtbh February 2, 2026 09:35

bgoral-rtbh approved these changes Feb 2, 2026

View reviewed changes

rtbhouse_sdk/api_tokens/managers.py Outdated Show resolved Hide resolved

rtbhouse_sdk/api_tokens/managers.py Outdated Show resolved Hide resolved

don't use duplicated manager methods, configure_from_env to not retur…

cb4f3ff

…n bool

jszymanski-rtbh requested a review from peku33 February 2, 2026 16:33

peku33 reviewed Feb 10, 2026

View reviewed changes

jszymanski-rtbh added 3 commits March 9, 2026 10:04

improvments after review

79aa9bf

fix imports

b6f2f64

add rotation window condition to keep-alive

83c6afc

jszymanski-rtbh force-pushed the APPS-14746-api-tokens-in-sdk branch from 4b52b7e to 83c6afc Compare March 9, 2026 09:28

jszymanski-rtbh requested a review from peku33 March 9, 2026 09:44

peku33 requested changes Mar 12, 2026

View reviewed changes

		def _post_dict(
		self, path: str, data: dict[str, Any] \| None = None, params: dict[str, Any] \| None = None

		return schema.RotatedApiToken(**data)


		class _HttpxApiTokenAuth(httpx.Auth):

Conversation

jszymanski-rtbh commented Feb 2, 2026

Uh oh!

jszymanski-rtbh commented Feb 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peku33 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants