Low code CDK: Make refresh token in oauth authenticator optional #26031
Conversation
There was a problem hiding this comment.
does it make sense to encode this via JSONSchema dependency keywords? https://json-schema.org/understanding-json-schema/reference/conditionals.html
| @@ -22,7 +22,7 @@ def __init__( | |||
| token_refresh_endpoint: str, | |||
| client_id: str, | |||
| client_secret: str, | |||
| refresh_token: str, | |||
There was a problem hiding this comment.
why is the type annotation removed?
There was a problem hiding this comment.
That was an oversight, I will add it back
|
@sherifnada JSON schema isn't great on types that depend on values of other fields - I tested around with if-then-else blocks (https://json-schema.org/understanding-json-schema/reference/conditionals.html#id6) but couldn't make that work - the approach https://stackoverflow.com/questions/9029524/json-schema-specify-field-is-required-based-on-value-of-another-field should work but it's not compatible with how we generate docs and types out of our schema. I think ultimately it would be nice to express this invariant in the schema but it's not worth the downsides based on current tooling |
|
@girarda added back that annotation, could you take another look? |
…bytehq#26031) * make refresh_token optional * format * add back type annotation
What
Fixes #23450
How
Makes the refresh_token parameter optional in schema and python classes, add test to fail if it's not set for grant_type "refresh_token".