-
Notifications
You must be signed in to change notification settings - Fork 294
feat(python-sdk): support omitting username/password from basic auth when configured in IR #14407
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
97ba837
a3f8635
e388f2c
5167926
1d56f47
af66447
d447854
f0f03d5
55b8f55
0e7aed7
b68310c
dea51d2
b85372b
e42b9fe
87deef3
dd520ad
b2e22fd
406c776
96607ee
ad58c5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴
as unknown as Record<string, unknown>violates CLAUDE.md rule and relies on fragile data smugglingCLAUDE.md explicitly prohibits
as unknown as Xtype assertions: "Never useas anyoras unknown as X. These are escape hatches that bypass the type system entirely. If the types don't line up, fix the types."Beyond the rule violation, this pattern exists because
usernameOmit/passwordOmitare smuggled as extra properties on aDynamicSnippets.BasicAuthobject (which only definesusernameandpasswordperpackages/ir-sdk/src/sdk/api/resources/dynamic/resources/auth/types/BasicAuth.ts:5-8). The converter atpackages/cli/generation/ir-generator/src/dynamic-snippets/DynamicSnippetsConverter.ts:736-749attaches these as ad-hoc properties, relying on JavaScript spread to propagate them. If the dynamic snippet IR passes through any schema-based serialization/deserialization boundary (as is typical when IR is passed to generators via JSON), these undeclared fields may be silently stripped, causing!!authRecord.usernameOmitto always evaluate tofalseand the feature to silently not work. The proper fix is to extend theDynamicSnippets.BasicAuthtype in the IR definition to include the new fields.Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a known limitation. The
FernIr.dynamic.BasicAuthtype comes from@fern-fern/ir-sdk(the published IR SDK package), which doesn't have typedusernameOmit/passwordOmitfields yet. The fields exist in the IR schema (packages/ir-sdk/src/sdk/api/resources/auth/types/BasicAuthScheme.ts) but the dynamic IR types haven't been updated to include them. Updating the IR types is out of scope for this PR per maintainer instruction ("Fix the non-IR changes"). The cast is necessary until the published IR SDK is updated.