Announce new features and version compatibility check #16
Conversation
davidotte
requested review from
SamuelGabriel,
noahho,
anshulg954 and
liam-sbhoo
liam-sbhoo
changed the title
liam-sbhoo
changed the title
davidotte
force-pushed
the
announce-new-features
branch
from
f530d56 to
2dd9fec
Compare
|
Hey @SamuelGabriel @liam-sbhoo , I just rebased to main and applied the requested changes :) |
tabpfn_client/client.py
Outdated
| @@ -251,6 +278,7 @@ def login(self, email: str, password: str) -> str | None: | |||
| data=common_utils.to_oauth_request_form(email, password) | |||
| ) | |||
|
|
|||
| self._validate_response(response, "login", only_version_check=True) | |||
There was a problem hiding this comment.
please set only_version check to false here
|
This all looks nice! this is good to be merged (without another look) after you:
Thanks :) |
|
@SamuelGabriel Done. |
tabpfn_client/client.py
Outdated
| load = None | ||
| try: | ||
| load = response.json() | ||
| except Exception: |
There was a problem hiding this comment.
Sorry I forgot to mention this, could we only catch known Exception and log the non-expected one? It seems a little dangerous.
I got the following from ChatGPT:
try:
response_payload = response.json()
except json.JSONDecodeError as e:
logging.error(f"Failed to parse JSON from response in {method_name}: {e}")| @@ -27,6 +28,14 @@ def test_try_connection_with_invalid_server(self, mock_server): | |||
| mock_server.router.get(mock_server.endpoints.root.path).respond(404) | |||
| self.assertFalse(self.client.try_connection()) | |||
|
|
|||
| @with_mock_server() | |||
| def test_try_connection_with_outdated_client(self, mock_server): | |||
There was a problem hiding this comment.
In general, a better practice for the naming the test cases should include:
- test condition
- expected result
In this case, you could call it def test_try_connection_with_outdated_client_raises_xxx
ps. I figured that the old test cases were following this naming convention, but let's start applying this now :)
Update client for retrieving new messages of a user and supporting the client version compatibility check of the server.
More detail:
I rebased to the fix_tests_client branch from Anshul, which is not merged yet.