fix: raise the typed errors, and make the smoke suite exercise them - #42
Merged
Conversation
Typed errors were dead code. `flat_api/errors.py` defined FlatNotFoundError,
FlatRateLimitError and the rest along with a from_response mapper, and nothing ever
called it: api_client.py still raised the generated ApiException subclasses. A developer
following the README and catching FlatNotFoundError caught nothing. Typed errors are most
of why an SDK is worth using over raw HTTP, and it is listed as a headline feature of the
2.0.0 rebuild.
tools/patches/20_errors.py wrote the module but never rewired the one raise site. It does
now, placing the import with the other first-party ones rather than above the module
docstring, and it is idempotent.
The smoke suite is what found this, on its first real run:
FAIL typed-not-found: raised NotFoundException, not FlatNotFoundError
FAIL typed-auth-error: raised UnauthorizedException, not FlatAuthenticationError
Three fixes to the runner itself, all from running it rather than reading it. `data` must
be base64: `dataEncoding` accepts nothing else, so the raw text failed validation in the
client. `createScore` takes the ScoreCreation union rather than a variant directly. And
the pagination scenario now traverses listCollections rather than getUserScores, because
the latter returns only public scores and the score this run creates is private, so it
traversed an empty list and proved nothing.
The SDK_RELEASE_TOKEN assertion in tag-on-merge.yml moved above the checkout step. Below
it, a missing secret surfaced as "Input required and not supplied: token", which names
neither the secret nor why it cannot be the automatic GITHUB_TOKEN.
Verified against production: whoami, create, read, update, export, paginate, both typed
errors, and cleanup deleting the score it created.
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #41, found by running the smoke suite against production for the first time.
Typed errors were dead code
flat_api/errors.pydefinedFlatNotFoundError,FlatRateLimitErrorand the rest, plus afrom_responsemapper. Nothing called it.api_client.pystill raised the generatedApiExceptionsubclasses, so a developer following the README and catchingFlatNotFoundErrorcaught nothing.The smoke suite said so on its first real run:
tools/patches/20_errors.pywrote the module but never rewired the raise site. It does now, and stays idempotent.Worth noting every other check passed while this was broken: coverage, docs, determinism, idempotency, the build matrix. Only a real request against a real API could catch it, which is the argument for the smoke suite existing.
Three runner fixes, all from running it
datamust be base64.dataEncodingaccepts nothing else, so raw text failed validation in the client.createScoretakes theScoreCreationunion, not a variant directly.listCollectionsrather thangetUserScores: the latter returns only public scores and the score this run creates is private, so it traversed an empty list and proved nothing.Also
The
SDK_RELEASE_TOKENassertion intag-on-merge.ymlmoved above the checkout step. Below it, a missing secret surfaced asInput required and not supplied: token, naming neither the secret nor why it cannot be the automaticGITHUB_TOKEN.Verified
Against production, with cleanup:
Patch idempotency passes.