Skip to content

feat: multipart upload #42

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: multipart upload #42

wants to merge 11 commits into from

Conversation

WaylonFtrack
Copy link

@WaylonFtrack WaylonFtrack commented May 27, 2024

Resolves FT-2fc3d98e-2756-45f4-b39f-3ddbef956a2f

  • I have added automatic tests where applicable.
  • The PR contains a description of what has been changed.
  • The description contains manual test instructions.
  • The PR contains updates to the release notes.
  • I have verified that the documentation is still up to date.

Changes

Using httpx as async HTTP library for upload request.

Test

@WaylonFtrack WaylonFtrack requested a review from a team as a code owner May 27, 2024 03:07
for i in range(retries):
try:
return await func(*args)
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps a bit broad? Maybe limit it to httpx.TimeoutException and httpx.HTTPStatusError in combination with exception.response.is_server_error()

if self.parts_count > MAX_PARTS:
raise ValueError("File is too big.")

self.upload_id: Optional[str] = None
Copy link
Contributor

Choose a reason for hiding this comment

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

these are only used within start perhaps they do not need to be instance variables?

@@ -81,7 +81,7 @@ def flush(self):

def seek(self, offset, whence=os.SEEK_SET):
"""Move internal pointer by *offset*."""
self.wrapped_file.seek(offset, whence)
return self.wrapped_file.seek(offset, whence)
Copy link
Contributor

Choose a reason for hiding this comment

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

consider updating parent class doc string to indicate that position is returned.

raise ValueError("Invalid file size.")


async def back_off(func: Callable[..., Awaitable], *args, retries=5, delay=5):
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe include typing where possible ie

async def back_off(func: Callable[..., Awaitable], *args:typing.Any, retries:int=5, delay:int=5):


startPos = (part_num - 1) * self.chunk_size
self.file.seek(startPos)
content = self.file.read(self.chunk_size)
Copy link
Contributor

Choose a reason for hiding this comment

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

we are reusing the same file object across all tasks, I suppose that is fine since we are reading / seeking synchronously?

Copy link
Author

Choose a reason for hiding this comment

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

yeah, we are reading synchronously and are reading the content before dropping it to the upload task.

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