-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add download and download_file methods #5
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
Conversation
src/wokwi_client/client.py
Outdated
Returns: | ||
The response message from the server. | ||
""" | ||
return await download(self._transport, name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just returned the bytes directly, as returning ResponseMessage isn't very useful for users of this API (they either want to bytes or to save the file, they shouldn't care about the format of the repsonse / or the base64 encoded data)
src/wokwi_client/file_ops.py
Outdated
return await transport.request("file:download", {"name": name}) | ||
|
||
|
||
async def download_file(transport: Transport, name: str, local_path: Optional[Path] = None) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about removing download_file
and moving the logic to the API client?
The idea is that most users won't use the file_ops.py
, as these are the low level wrappers for the API calls. I don't have a strong opinion here - it also depends on what solution we end up using for the sync client (if we end up duplicating the WokwiClient code, then it's better to keep more logic in the file_ops to reduce the amount of duplicated code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have found a way to create the wrapper based on an async client without duplicating the code. So I have moved the download_file logic to the API client.
Looks like the WokwiClient download/download_files method has disappeared in the last commit? |
Fixed |
This pull request adds file download support to the Wokwi client, complementing the existing upload functionality. The main changes introduce new methods to download files from the simulator, both as raw responses and directly to the local filesystem.