Accept the TT_* key spellings in credentials.json#10
Merged
Conversation
The loader only accepted snake_case keys (client_id, client_secret, refresh_token), but the natural thing to write is the TT_* names, which is what .env.example, the environment, and the Tastytrade docs all use. A file holding TT_CLIENT_ID / TT_SECRET / TT_REFRESH was reported as missing all three credentials, which reads as "no credentials" rather than "wrong key names". - Both spellings are accepted per field, snake_case preferred when both appear. base_url likewise accepts API_BASE_URL. - The missing-key error now names both spellings and lists which keys the file actually contained, so a naming mismatch is diagnosable from the message. - TT_USERNAME and TT_PASSWORD are ignored: the OAuth refresh-token grant reads neither. When they are the only keys present, the error says so, since they are otherwise secrets at rest that nothing reads. Verified against a real credentials.json using the TT_* names: it resolves, and a live read-only list_accounts through the plugin launcher returns the account. Values are never echoed in any error; a test asserts that with distinctive sentinel values. Version 0.2.0 so installed copies pick this up.
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
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.
The loader I shipped in #9 only accepted snake_case keys, so a credentials file written with the
TT_*names failed. Reported live as:The file had all three values, just under the names used everywhere else. My design error: I mirrored harbor's key style without checking what the values are actually called in this project.
Fix
Both spellings accepted per field, snake_case preferred when both appear:
client_idTT_CLIENT_IDclient_secretTT_SECRETrefresh_tokenTT_REFRESHbase_urlAPI_BASE_URLThe
TT_*forms are what.env.example, the environment, and the Tastytrade docs all use, so a file written from any of those is the obvious thing to produce and must not be rejected.The missing-key error now names both spellings and lists which keys the file actually contained, so a naming mismatch is diagnosable from the message rather than reading as "no credentials at all".
TT_USERNAMEandTT_PASSWORDare ignored: the OAuth refresh-token grant reads neither. When they are the only keys present the error says so, since they are otherwise secrets sitting at rest that nothing reads.Verification
Against a real
credentials.jsonusing theTT_*names:A live read-only
list_accountsthrough the plugin launcher returns the account.Four new tests cover the aliases, the both-spellings precedence, ignored username/password keys, and that the error names both spellings while never echoing a value (asserted with distinctive sentinels).
Two notes from debugging this
A transient HTTP 400 appeared on one
list_accountscall and did not reproduce; the same call succeeds directly and through the launcher. Along the way I suspected two things and confirmed both were not bugs, rather than "fixing" working code:authenticate()interpolates the form body without URL-encoding. All three values are alphanumeric plus-_., so nothing needs encoding. Still worth knowing it would break on a secret containing+or/.Content-Type: application/jsonon bodyless GETs. The API returns 200 either way.Version 0.2.0 so installed copies receive this.