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

Fix missing mode property on file wrapper #2495

Merged
merged 2 commits into from
Sep 19, 2022

Conversation

oefe
Copy link
Contributor

@oefe oefe commented Aug 27, 2022

Add the missing mode property to the file wrapper used by rich.progress.wrap_file.

This fixes an incompatibility with the requests library.

Type of changes

  • Bug fix
  • New feature
  • Documentation / docstrings
  • Tests
  • Other

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review.

Description

Without this property, uploads using requests fail to set the Content-Length header, and fall back to chunked encoding,
which many hosts (e.g. S3) don't support.

requests uses the mode property in its
super_len
function, which determines the length of the stream to upload.

Example Code (Fragment)

def upload_stream(session: requests.Session, source: BinaryIO, remote_url: str, size: int, label: str):
    """Upload a data stream from `source `to `remote_url``."""
    with wrap_file(source, size, description=label) as f:
        response = session.put(
            url=remote_url,
            data=f,
        )
   response. raise_for_status()


def upload_file(session: requests.Session, source: pathlib.Path, remote_url: str):
    """Upload a file from `source `to `remote_url``."""
    with source.open(mode="rb") as f:
        upload_stream(
            session,
            f,
            remote_url=remote_url,
            size=source.stat().st_size,
            label=source.name,
        )

Before fix

Request headers

User-Agent: python-requests/2.28.1
Accept-Encoding: gzip, deflate
Accept: */*
Connection: keep-alive
Transfer-Encoding: chunked

Response

501 Not Implemented
x-amz-request-id: ...
x-amz-id-2: ...
Content-Type: application/xml
Transfer-Encoding: chunked
Date: Sat, 27 Aug 2022 14:57:45 GMT
Server: AmazonS3
Connection: close
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>NotImplemented</Code><Message>A header you provided implies functionality that is not implemented</Message><Header>Transfer-Encoding</Header><RequestId>...</RequestId><HostId>...=</HostId></Error>

After fix

Request headers

User-Agent: python-requests/2.28.1
Accept-Encoding: gzip, deflate
Accept: */*
Connection: keep-alive
Content-Length: 9701

Response

200 OK
x-amz-id-2: ...
x-amz-request-id: ...
Date: Sat, 27 Aug 2022 14:43:32 GMT
ETag: "..."
Server: AmazonS3
Content-Length: 0

oefe added 2 commits August 27, 2022 17:08
Without this property, uploads using `requests` fails to set the
Content-Length header, and falls back to chunked encoding,
which many hosts (e.g. S3) don't support.
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

Successfully merging this pull request may close these issues.

2 participants