Api devices#209
Conversation
non-Jedi
left a comment
non-Jedi
left a comment
There was a problem hiding this comment.
Other than nits, LGTM. Thanks!
|
|
||
| def update_device_info(self, device_id, display_name): | ||
| """Update the display name of a device. | ||
| Args: |
There was a problem hiding this comment.
Please include a blank line after every first line summary of a docstring.
|
|
||
| def delete_device(self, auth_body, device_id): | ||
| """Deletes the given device, and invalidates any access token associated with it. | ||
| Requires Auth. |
There was a problem hiding this comment.
Should probably explicitly say that it uses the "User-Interactive Authentication API." Lots of endpoints require auth, but very few use the user-interactive flow. Also should be separated from the "Args" block by an empty line I believe.
| @responses.activate | ||
| def test_delete_device(self): | ||
| delete_device_url = "http://example.com/_matrix/client/r0/devices/QBUAZIFURK" | ||
| responses.add(responses.DELETE, delete_device_url, body='{}') |
There was a problem hiding this comment.
Can we test that things behave as expected when receiving 401 responses since that's part of the user-interactive auth api?
|
Oh, and signoff please. :) |
|
Should be good now. |
There was a problem hiding this comment.
Sorry I missed this the first time through. Please try to follow PEP 257 conventions for multi-line docstrings. In this case, that means a single summary line separated by a blank new line from a more detailed description. Something like:
"""Deletes the given device and invalidates any associated access tokens.
NOTE: This endpoint uses the User-Interactive Authentication API.
Args:
auth_body (dict): Authentication params.
device_id (str): The device ID of the device to delete.
"""There was a problem hiding this comment.
Thanks, fixed. I mostly know PEP8 through flake8 and tend to miss those kind of things.
3 participants
Not sure if I should squash pik's commit and my fixes.
Tests felt really repetitive to write, I wonder if there isn't some way to refactor them (maybe using some pytest magic). edit: just saw that there have already been discussions on this and that pik had started working on the issue, I should see if I can take the time to look into this.