feat!: rebuild the client as 2.0.0 against API 2.26.0 - #41
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
e3f36cc to
df91280
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Caution Review the following alerts detected in dependencies. According to your organization's Security Policy, you must resolve all "Block" alerts before proceeding. Learn more about Socket for GitHub.
|
13b2f85 to
e4d324d
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f36cce09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # shellcheck source=tools/lib/generate-common.sh | ||
| source tools/lib/generate-common.sh |
There was a problem hiding this comment.
The new generation entrypoint exits immediately because tools/lib/generate-common.sh is not present anywhere in the repository. This prevents the documented tools/generate.sh command from running and also breaks the determinism/idempotency jobs that invoke regeneration; commit the helper or make this script self-contained.
Useful? React with 👍 / 👎.
| self.tokens = TokenManager(tokens, helper=oauth, on_token_refresh=on_token_refresh) | ||
| self.retry = retry if retry is not None else RetryPolicy() | ||
| self.base_url = base_url | ||
| self.timeout = timeout | ||
|
|
||
| configuration = Configuration(host=base_url) | ||
| configuration.access_token = self.tokens.access_token | ||
| self._configuration = configuration | ||
| self._api_client = self._build_api_client(configuration) |
There was a problem hiding this comment.
When requests are made through FlatClient or AsyncFlatClient, these values are only stored on the wrapper: the generated ApiClient receives a configuration containing the token but never receives self.timeout or self.retry, and no request path invokes _reauthenticate or errors.from_response. Consequently the advertised default timeout, retries, typed errors, and transparent refresh are all inactive; generated operations retain their own timeout defaults and raise generated ApiException subclasses.
Useful? React with 👍 / 👎.
| # 1. The token authenticates. | ||
| print(" whoami ... ", end="") | ||
| client.api_client # noqa: B018 - construction proves configuration is valid | ||
| print("ok") |
There was a problem hiding this comment.
Even with an invalid but nonempty FLAT_TEST_TOKEN, the authentication scenario performs no network request and unconditionally reports success. The score lifecycle similarly only creates a title, pagination uses a local empty lambda, and typed-error checks only inspect inheritance, so this script can print smoke: PASS while authentication and every real SDK operation are broken.
Useful? React with 👍 / 👎.
| @@ -1,5 +0,0 @@ | |||
| pytest~=7.1.3 | |||
There was a problem hiding this comment.
Deleting this file makes every existing job in .gitlab-ci.yml fail at pip install -r test-requirements.txt before tests can run. The same pipeline also still targets Python 3.7–3.10 despite the new requires-python >=3.11, so the GitLab status described as required by the new GitHub workflow cannot succeed until that pipeline is updated or this file is retained.
Useful? React with 👍 / 👎.
| repository: FlatIO/api-reference | ||
| ref: ${{ vars.SPEC_REF || 'v2.25.1' }} |
There was a problem hiding this comment.
When the repository variable is unset, the checks job fetches v2.25.1 even though this commit's generated sources and OPERATIONS.json declare API 2.26.0. The coverage/docs checker and regeneration checks therefore compare different API surfaces and can fail or regenerate a different tree; default this checkout to v2.26.0.
Useful? React with 👍 / 👎.
| with self._lock: | ||
| current = self._tokens.access_token | ||
| if self._tokens.access_token != current: | ||
| return self._tokens.access_token |
There was a problem hiding this comment.
When multiple threads call refresh() for the same expired token, each waiter acquires the lock in turn and sets current from the already-current token, making the following comparison tautologically false. Every waiter therefore performs another refresh instead of reusing the first result, which can rotate credentials repeatedly or invalidate the token returned to the first request; capture the expected stale token before waiting or otherwise track an in-flight refresh generation.
Useful? React with 👍 / 👎.
44eff9c to
9c6eb8f
Compare
Replaces the 1.x client, generated from Swagger Codegen against a 2020-era specification, with one regenerated from api-reference v2.26.0 through a pipeline that reproduces from this repository alone. 123 operations, the full public surface of API 2.26.0, against 119 before. `FlatClient` and `AsyncFlatClient`: the generator emits sync or async and never both, so tools/generate.sh runs it twice and merges the async client under `flat_api.aio`. Typed errors, a retry layer that understands Flat's rate limiting (HTTP 403 with the reset in `X-RateLimit-Reset`, not 429), cursor pagination that reads the undeclared `Link` header, `OAuth2Helper`, and a `py.typed` marker so a consumer's type checker stops ignoring the annotations. `SPEC_REF=v2.26.0 ./tools/generate.sh` fetches the specification by tag, runs openapi-generator pinned to an exact 7.24.0, wipes the zone `.sdkgen.yaml` declares generated, applies the post-generation patches and writes OPERATIONS.json. Two checks guard it: determinism, that the same inputs twice produce identical trees, and idempotency, that re-applying every patch changes nothing. Packaging was broken in two ways that a green build hid. `uv build` emitted `flat_api-0.0.0` and exited 0, because pyproject.toml still carried a `[tool.poetry]` table under a setuptools backend, so setuptools saw no `[project]` table and fell back to directory discovery for the name and 0.0.0 for the version; the only guard compared the tag against `cat VERSION`, which the build never read. And the dependency list omitted `httpx`, `aiohttp` and `aiohttp-retry`, all of which `flat_api/__init__` reaches through its own re-exports, so a clean install could not `import flat_api` at all. It is now a PEP 621 project with the version derived from VERSION, Apache-2.0 declared with its text, and `requires-python >=3.11` matching the tested matrix. The release-please workflows are removed. They published on every push to master with a long-lived token, taking their version from release-please's manifest rather than VERSION, which bypassed the merge gate, the tag, OIDC and provenance. Publishing is now the tag alone: tag-on-merge.yml creates it with SDK_RELEASE_TOKEN, since GitHub does not start a workflow run for an event created with the automatic GITHUB_TOKEN, and release.yml uploads to PyPI by trusted publishing with attestations. Verified: import and compileall on 3.11, 3.12 and 3.13; a clean install of the built wheel imports FlatClient, AsyncFlatClient and OAuth2Helper; the wheel carries py.typed, flat_api/aio/ and LICENSE and no tests or docs; 123 of 123 operations and 23 scopes with no internal leak; 100% documentation coverage; determinism, idempotency and zone checks pass; and all three LTI 1.3 modes deserialize to their own variant.
9c6eb8f to
38a7c7d
Compare
1 participant
Replaces the 1.x client, generated from Swagger Codegen against a 2020-era specification, with a client regenerated from api-reference v2.26.0 and a reproducible pipeline.
This one has to be merged by hand. The regeneration pipeline clones this repository's default branch and reads
.sdkgen.yamlfrom it, and master has no such file. So the pipeline cannot produce this pull request; every release after it can.What lands
FlatClientandAsyncFlatClient. The generator emits sync or async and never both, sotools/generate.shruns it twice and merges the async client in underflat_api.aioOAuth2HelperandTokenManagerLinkheaderpy.typed, so a consumer's type checker stops ignoring the annotationsReproducible from this repository alone
tools/generate.shfetches the specification by tag, runs the pinned generator (openapi-generator 7.24.0, exact, no range), wipes the generated zone declared in.sdkgen.yaml, applies the post-generation patches and writesOPERATIONS.json. No orchestrator and no sibling checkout needed:Two checks guard it: determinism, that the same inputs twice produce identical trees, and idempotency, that re-applying every patch changes nothing.
Packaging, which was broken
uv buildused to emitflat_api-0.0.0and exit 0. pyproject.toml still carried a[tool.poetry]table under a setuptools backend, so setuptools saw no[project]table, fell back to directory discovery for the name and 0.0.0 for the version. The only guard compared the tag againstcat VERSION, which the build never read.The dependency list was wrong in a way that made the package unimportable:
flat_api/__init__re-exportsOAuth2Helper(which importshttpx) andFlatClient(which pulls inflat_api.aio, soaiohttpandaiohttp-retry), none of them declared. A cleanpip install flat-apifollowed byimport flat_apiraisedImportError.Now a PEP 621 project with the version derived from
VERSION, Apache-2.0 declared with its text,requires-python >=3.11matching the tested matrix, and every import declared.Verified
requirements.txt, import,compilealluv buildemitsflat_api-2.0.0; a clean install importsFlatClient,AsyncFlatClient,OAuth2Helperpy.typed,flat_api/aio/and LICENSE, no tests or docstools/,.github/or.sdkgen.yamlBefore merging
Merging creates the tag, and the tag is what publishes. Two things must exist first, or the release stops:
SDK_RELEASE_TOKEN, a repository secret withcontents: write.tag-on-merge.ymlpushes the tag with it and asserts it is set. It cannot use the automaticGITHUB_TOKEN: GitHub does not start a workflow run for an event created with that token, so the tag would appear andrelease.ymlwould never fire.pypienvironment and trusted publishing configured on PyPI for this repository andrelease.yml. The workflow filename is part of the trust and cannot be renamed afterwards.GITLAB_READ_TOKENis also needed for thechecksjob, which clones the checker.Once trusted publishing is live, the old
PYPI_API_TOKENsecret can be revoked: the release-please workflows that used it are removed here, and nothing else reads it.