Conversation
renceInbox
force-pushed
the
enhancement/add-test-and-coverage
branch
2 times, most recently
from
a36010c to
dd789cf
Compare
renceInbox
force-pushed
the
enhancement/add-test-and-coverage
branch
from
dd789cf to
4bd0ee7
Compare
.github/workflows/CI.yaml
Outdated
| pre-commit: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2 |
There was a problem hiding this comment.
@renceInbox can we use the latest actions/checkout, which is v3 here? It is used further down already.
openspp_dms_ftp_server/clients.py
Outdated
| if response.status_code != 200: | ||
| raise OpenSPPClientException(f"Unsuccessful login. Response data: {data}") | ||
| data = response.json() | ||
| result = response.json().get("result", "") |
There was a problem hiding this comment.
Isn't this check redundant as the code is already checking response.status_code and raising an exception if it is != 200?
There was a problem hiding this comment.
@renceInbox I see that the test test_unsuccessful_authentication is checking the contents of the JSON response and even if the result is 200, the contents of the JSON seems to indicate that there is still an error. Can you please remind me what the source of this error could be?
There was a problem hiding this comment.
Hi @kneckinator , this could happen in login. If we provide an invalid username and password the response will be 200 but the result will say 401.
openspp_dms_ftp_server/clients.py
Outdated
| data = response.json() | ||
| if not response.status_code == 200: | ||
| if response.status_code != 200: | ||
| raise OpenSPPClientException(f"Unsuccessful login. Response data: {data}") |
There was a problem hiding this comment.
I believe that the data in the exception should be response.json(). As written now, it will print the request data.
There was a problem hiding this comment.
Hi @kneckinator , I fixed the variable names to avoid this.
2 participants
No description provided.