-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: add version check support #299
base: main
Are you sure you want to change the base?
Conversation
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
@@ -48,6 +48,7 @@ def test_posit_credentials_provider(self): | |||
register_mocks() | |||
|
|||
client = Client(api_key="12345", url="https://connect.example/") | |||
client.ctx.version = None |
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.
We are setting the version to None
to avoid mocking the API request to /server_settings.
After a few iterations, I landed on the decorator approach. This seems idiomatic and self-documenting. I tried a few declarative approaches (i.e., checking the version inline), but that felt brittle. |
con = Client(api_key="12345", url="https://connect.example/") | ||
assert con.oauth.get_credentials("cit")["access_token"] == "viewer-token" | ||
c = Client(api_key="12345", url="https://connect.example/") | ||
c.ctx.version = None |
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.
Should we test here that the oauth client resource respects the server's version constraint?
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.
Good call. I'll add it.
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.
Looks great! I like the annotation approach, that feels very idiomatic
Adds a new decorator,
@context.requires
, which asserts version compatibility when the server version is known. The check is skipped if the server version is unknown (e.g., the Connect configuration disables version information).Also marks the OAuth API with a '2024.08.0' requirement.
Closes #272