devopshq / tfs

Microsoft TFS API Python client
https://devopshq.github.io/tfs/
MIT License
110 stars 28 forks source link

HTTP 201 (Created) being treated as an error #97

Closed aderbas closed 2 years ago

aderbas commented 2 years ago

I'm trying to use the send_post() method to upload an attachment to attach to wit, but the method is treating 201 as an error.

# connect OK

real_path = os.path.join(os.path.dirname(__file__), 'final_log.lst')
with open(real_path, 'rb') as file:
    response = client.rest_client.send_post(
        'wit/attachments?fileName=textAsFileAttachment.lst&api-version=1.0',
        data=f"[{file.read()}]",
        headers={'Content-Type': 'application/octet-stream'},
    )
    print(response)
Traceback (most recent call last):
  File "/home/aderbal/Workspace/assert/test-tfs-conn.py", line 86, in <module>
    response = client.rest_client.send_post(
  File "/home/aderbal/.local/lib/python3.10/site-packages/tfs/connection.py", line 456, in send_post
    return self.__send_request('POST', uri, data, headers, payload=payload, underProject=project)
  File "/home/aderbal/.local/lib/python3.10/site-packages/tfs/connection.py", line 518, in __send_request
    raise TFSClientError('TFS API returned HTTP %s (%s)' % (
tfs.connection.TFSClientError: TFS API returned HTTP 201 (Created)

The API returns 201 - Created and is being treated as an error but 201 is a success status. I've been looking in the source code and found it.

526 - connection.py

       if response.status_code != 200:
            raise TFSClientError('TFS API returned HTTP %s (%s)' % (
                response.status_code, result['error'] if 'error' in result else response.reason))
        return result
aderbas commented 2 years ago

I suggest changing it to:

                result = response.json()

                if response.status_code not in (200, 201):
                    raise TFSClientError('TFS API returned HTTP %s (%s)' % (
                        response.status_code, result['error'] if 'error' in result else response.reason))
                return result
allburov commented 2 years ago

@aderbas hi! We can use response.raise_for_status() and catch all exceptions from it. Feel free to create a PR with the changes!

aderbas commented 2 years ago

I really need the return from the request, the TFS API responds with the id and url of the file. Even if I treat the error, I won't have this information. I opened a PR but there was an error in the pre-commit, I don't understand the cause, sorry about that.

Boltyk commented 2 years ago

@allburov is there a sandbox or something to play with TFS API? I don't have TFS anymore

allburov commented 2 years ago

@Boltyk I don't think so, neither do I have access :(

@aderbas thank you for the contribution! I'll release a new version after migration to Github Actions https://github.com/devopshq/tfs/issues/99

allburov commented 2 years ago

https://pypi.org/project/dohq-tfs/1.1.1/