Conversation
|
@mattgd - I'd like to get this contribution in, but I'm unsure of the next steps. The PR template tells me link a PR to corresponding docs, however, searching for |
nholden
left a comment
There was a problem hiding this comment.
👋 Hey, @denizs, thanks for this contribution!
I pulled this PR down and tried getting it working with an example app locally, and I bumped into some issues. First, I saw a ModuleNotFoundError: No module named 'workos.types.api_keys', which Claude Code suggests could reflect a naming conflict between a type and a class. Second, I saw that the API endpoint wasn't being called correctly, as I noted inline.
I'd like to get this contribution in, but I'm unsure of the next steps.
Could you get this change working in an app on your machine? After you've done that, please ping me for another review.
|
Hey @nholden sorry for that very unpleasant review. I should've mentioned the PR is by far not ready for review and that I'm primarily wondering what to do about the following sentence in the PR template:
It sounded like I had to first create another PR somewhere else before getting this one reviewed. I've cleaned up the AI slop and can confirm it's working on my machine ™️. I've used the API key widget to issue an API key for our application and then successfully validated it: |
Greptile OverviewGreptile SummaryThis PR adds API key validation functionality to the workos-python SDK, allowing developers to programmatically validate API keys. Key Changes:
Implementation Quality:
Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Client as SDK Client
participant ApiKeys as ApiKeys Module
participant HTTP as HTTP Client
participant API as WorkOS API
Client->>ApiKeys: validate_api_key(value="sk_...")
ApiKeys->>HTTP: request(path="api_keys/validations", method=POST, json={value: "sk_..."})
HTTP->>API: POST /api_keys/validations
alt Valid API Key
API-->>HTTP: 200 OK {api_key: {...}}
HTTP-->>ApiKeys: response_json
ApiKeys->>ApiKeys: ApiKey.model_validate(response["api_key"])
ApiKeys-->>Client: ApiKey object
else Invalid API Key
API-->>HTTP: 401 Unauthorized
HTTP->>HTTP: raise AuthenticationException
HTTP-->>ApiKeys: AuthenticationException
ApiKeys-->>Client: AuthenticationException
end
|
nholden
left a comment
There was a problem hiding this comment.
Thanks, @denizs! Left a couple of questions for you.
I'm primarily wondering what to do about the following sentence in the PR template:
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.It sounded like I had to first create another PR somewhere else before getting this one reviewed.
Sorry, WorkOS needs to fix this! You did everything right. Our docs aren't open source, so I can make the docs change for you after we release this change.
|
Thanks @nholden! And sorry for the slight delay - I was on vacation |
mattgd
left a comment
There was a problem hiding this comment.
Looks great! Thanks for adding this and apologies for the delay.
Description
Documentation
Once a new version is released, one could update the code examples that are currently limited to
authkit-nextjsandexpress.If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.