Skip to content

Added merge environmental settings to prepared request #1811

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Sep 24, 2024
Merged

Added merge environmental settings to prepared request #1811

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
15519dc
added merge environmental settings to prepared request
Gabefire Sep 12, 2024
53197f8
feedback
Gabefire Sep 12, 2024
dd1a6e9
removed headers from request since gets overwritten on prepped request
Gabefire Sep 12, 2024
7322bfd
swapped back to just prepare
Gabefire Sep 12, 2024
4eadc80
formatted
Gabefire Sep 13, 2024
a3da2da
Merge branch 'develop' into gu/added_merge_setting
Gabefire Sep 13, 2024
cbc636b
Merge branch 'develop' into gu/added_merge_setting
Gabefire Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions libs/labelbox/src/labelbox/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,15 +214,20 @@ def convert_value(value):
if files:
del headers['Content-Type']
del headers['Accept']

request = requests.Request('POST',
endpoint,
headers=headers,
data=data,
files=files if files else None)

prepped: requests.PreparedRequest = request.prepare()

response = self._connection.send(prepped, timeout=timeout)
prepped: requests.PreparedRequest = self._connection.prepare_request(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not believe we should be using prepare_request on the Session itself... unfortunately so. This is because we are modifying headers above and if you use prepare_request on the session it will prb merge those headers back.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So just to test I printed out both the request headers and prepped headers

{
    "User-Agent": "python-requests/2.32.3",
    "Accept-Encoding": "gzip, deflate",
    "Accept": "application/json",
    "Connection": "keep-alive",
    "Authorization": "Bearer <API_KEY>",
    "Content-Type": "application/json",
    "X-User-Agent": "python-sdk 4.0.0",
    "X-Python-Version": "3.8.18-final",
}
{
    "User-Agent": "python-requests/2.32.3",
    "Accept-Encoding": "gzip, deflate",
    "Accept": "application/json",
    "Connection": "keep-alive",
    "Authorization": "Bearer <API_KEY>",
    "Content-Type": "application/json",
    "X-User-Agent": "python-sdk 4.0.0",
    "X-Python-Version": "3.8.18-final",
    "Content-Length": "733",
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I see what your saying. I modified it to add the correct headers to the prepped response. I believe that is what the docs recommended to allow the merge_environment_settings

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this back to just prepped but did the merge setting still

request
)

settings = self._connection.merge_environment_settings(prepped.url, {}, None, None, None)

response = self._connection.send(prepped, timeout=timeout, **settings)
logger.debug("Response: %s", response.text)
except requests.exceptions.Timeout as e:
raise labelbox.exceptions.TimeoutError(str(e))
Expand Down
Loading