Skip to content
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.

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

Update Requests library to latest #2704

Closed
phillxnet opened this issue Oct 15, 2023 · 6 comments
Closed

Update Requests library to latest #2704

phillxnet opened this issue Oct 15, 2023 · 6 comments
Assignees

Comments

@phillxnet
Copy link
Member

phillxnet commented Oct 15, 2023

Our prior pinned version of 'requests' can now be re-addressed re testing branch development concerning our base Python version, i.e. we now use Py3.9:

requests = "==2.27.1" # Last Python 2.7/3.6 version

Re-examine our options on updating this pinning or remove our pinning all-together. Note that there may well be additional parameters expected such as timeout etc. These should be addressed as part of this issue also. [EDIT] see below issue comment: #2704 (comment)

A potential enhancement of this update could be to enable server side compression compatibility that it is thought was introduced in an interim version of 'requests' post what were have pinned currently. This could help with the performance of such things as our Rock-on update mechanism. [EDIT] see below issue comment: #2704 (comment)

@phillxnet phillxnet self-assigned this Oct 15, 2023
@phillxnet
Copy link
Member Author

From the PyPi page: https://pypi.org/project/requests/

we look to no longer require a pinning, and given our dependency on django-oauth-toolkit has in-turn a dependency on this library, we are guaranteed to have it pulled in.

@phillxnet
Copy link
Member Author

Removing our existing pinning results in:

lbuildvm:/opt/rockstor # poetry update
Updating dependencies
Resolving dependencies... (2.1s)

Writing lock file

Package operations: 0 installs, 3 updates, 0 removals

  • Updating charset-normalizer (2.0.12 -> 3.3.0)
  • Updating urllib3 (1.26.17 -> 2.0.6)
  • Updating requests (2.27.1 -> 2.31.0)

@phillxnet
Copy link
Member Author

We import requests in the following files:

  • src/rockstor/cli/api_wrapper.py
  • src/rockstor/cli/rest_util.py
  • src/rockstor/system/pkg_mgmt.py
  • src/rockstor/storageadmin/views/rockon.py

@phillxnet
Copy link
Member Author

Re:

A potential enhancement of this update could be to enable server side compression compatibility that it is thought was introduced in an interim version of 'requests' post what were have pinned currently.

It seems that as part of the defualt header we already have this on the request side at least:

lbuildvm:/opt/rockstor # poetry run python
Python 3.9.18 (main, Sep 06 2023, 07:49:32) [GCC] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests
>>> response = requests.get('http://httpbin.org/headers')
>>> print(response.status_code)
200
>>> print(response.text)
{
  "headers": {
    "Accept": "*/*", 
    "Accept-Encoding": "gzip, deflate", 
    "Host": "httpbin.org", 
    "User-Agent": "python-requests/2.31.0", 
    "X-Amzn-Trace-Id": "Root=1-652c0e29-511c654c2be56d14303444fe"
  }
}

>>> exit()
lbuildvm:/opt/rockstor # 

Which is no different from what we were already advertising in the prior release:

...
>>> print(response.text)
{
  "headers": {
    "Accept": "*/*", 
    "Accept-Encoding": "gzip, deflate", 
    "Host": "httpbin.org", 
    "User-Agent": "python-requests/2.27.1", 
    "X-Amzn-Trace-Id": "Root=1-652c0f7f-5feb54a53a0b0f521fe9c4b9"
  }
}

Modifying original issue text accordingly.

@phillxnet
Copy link
Member Author

phillxnet commented Oct 15, 2023

Re:

Note that there may well be additional parameters expected such as timeout etc. These should be addressed as part of this issue also.

It appears we already have, and use, this facility where we allow 10 seconds for each Rock-on file retrieval already.
https://requests.readthedocs.io/en/latest/user/quickstart/#timeouts

  • 10 seconds for the root.json
  • 10 seconds for each file referenced in root.json

response = requests.get(remote_root, timeout=10)

and

cur_res = requests.get(cur_meta_url, timeout=10)

Modify original issue text accordingly.

@phillxnet phillxnet changed the title Update Requests library Update Requests library to latest Oct 15, 2023
phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Oct 15, 2023
Remove our prior pinning for 'requests' as no longer required.
## Includes:
- Adding 10 second timeout on update repo status check, to
avoid indefinite hang potential.
- Re Black formatting on changed pkg_mgmt.py file.
phillxnet added a commit that referenced this issue Oct 16, 2023
…to-latest

Update Requests library to latest #2704
@phillxnet
Copy link
Member Author

Closing as:
Fixed by #2705

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant