-
Notifications
You must be signed in to change notification settings - Fork 30
[WIP] add python paper muncher package #87
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
base: main
Are you sure you want to change the base?
Conversation
e292a04
to
deae482
Compare
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.
Pull Request Overview
This PR adds a comprehensive Python package for Paper Muncher, providing both synchronous and asynchronous interfaces for HTML-to-PDF rendering. The package includes framework integrations for Flask and WSGI applications, cross-platform I/O handling with timeout support, and comprehensive examples.
Key changes:
- Removes the old simple papermuncher.py implementation and sample.py
- Introduces a new structured package with synchronous and asynchronous rendering capabilities
- Adds framework-specific integrations for Flask and WSGI applications
Reviewed Changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 10 comments.
Show a summary per file
File | Description |
---|---|
meta/bindings/python/sample.py | Removed old sample implementation |
meta/bindings/python/papermuncher.py | Removed old monolithic implementation |
meta/bindings/python/paper_muncher/*.py | New package structure with typing definitions and binary utilities |
meta/bindings/python/paper_muncher/synchronous/*.py | Synchronous rendering implementation with cross-platform I/O |
meta/bindings/python/paper_muncher/asynchrous/*.py | Asynchronous rendering implementation |
meta/bindings/python/paper_muncher/runners/*.py | WSGI runner utilities |
meta/bindings/python/paper_muncher/frameworks/*.py | Flask and WSGI framework integrations |
meta/bindings/python/examples/*.py | Usage examples for different scenarios |
@@ -0,0 +1,92 @@ | |||
"""The :mod:`paper_muncher.synchronous.request` |
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.
The module docstring incorrectly references 'synchronous' instead of 'asynchronous' for an asynchronous module.
"""The :mod:`paper_muncher.synchronous.request` | |
"""The :mod:`paper_muncher.asynchrous.request` |
Copilot uses AI. Check for mistakes.
@@ -0,0 +1,278 @@ | |||
""" | |||
The :mod:`.paper_muncher.synchronous.interface` module provides |
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.
The module docstring incorrectly references 'synchronous' instead of 'asynchronous' for an asynchronous module.
The :mod:`.paper_muncher.synchronous.interface` module provides | |
The :mod:`.paper_muncher.asynchrous.interface` module provides |
Copilot uses AI. Check for mistakes.
@@ -0,0 +1,9 @@ | |||
"""The :mod:`paper_muncher.asynchronous` module |
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.
The module docstring references 'asynchronous' but the actual directory name is 'asynchrous' (misspelled). Either fix the directory name to 'asynchronous' or update the docstring to match.
"""The :mod:`paper_muncher.asynchronous` module | |
"""The :mod:`paper_muncher.asynchrous` module |
Copilot uses AI. Check for mistakes.
response_iterable, http_status, http_response_headers = run_wsgi_app( | ||
application, generate_environ(environ) |
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.
The function call passes 'environ' to 'generate_environ' but 'generate_environ' expects two parameters: 'path' and 'current_environ'. The 'request_path' parameter should be passed as the first argument.
response_iterable, http_status, http_response_headers = run_wsgi_app( | |
application, generate_environ(environ) | |
application, generate_environ(request_path, environ) |
Copilot uses AI. Check for mistakes.
timeout: int, | ||
) -> bytes: | ||
"""Read all data from a file-like object until EOF. | ||
|
||
:param BinaryIO file_object: File-like object to read from. | ||
:param int timeout: UNUSED timeout parameter. | ||
(Fallback implementation does not support timeouts) |
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.
The function signature is inconsistent with other implementations. The POSIX and NT versions include a 'chunk_size' parameter, but the fallback version does not. This could cause issues when switching between implementations.
timeout: int, | |
) -> bytes: | |
"""Read all data from a file-like object until EOF. | |
:param BinaryIO file_object: File-like object to read from. | |
:param int timeout: UNUSED timeout parameter. | |
(Fallback implementation does not support timeouts) | |
timeout: int, | |
chunk_size: int = 4096, | |
) -> bytes: | |
"""Read all data from a file-like object until EOF. | |
:param BinaryIO file_object: File-like object to read from. | |
:param int timeout: UNUSED timeout parameter. | |
(Fallback implementation does not support timeouts) | |
:param int chunk_size: UNUSED chunk size parameter. | |
(Fallback implementation does not support chunked reading) |
Copilot uses AI. Check for mistakes.
_logger.warning( | ||
"Using fallback readlines_with_timeout. " |
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.
The warning message mentions 'readlines_with_timeout' but the actual function name is 'read_all_with_timeout'.
_logger.warning( | |
"Using fallback readlines_with_timeout. " | |
"Using fallback read_all_with_timeout. " |
Copilot uses AI. Check for mistakes.
) % { | ||
b'length': len(content.encode()), | ||
b'date': format_datetime(now, usegmt=True).encode(), | ||
b'server': SERVER_SOFTWARE, | ||
} |
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.
The 'content' variable is expected to be a BytesIO object, but '.encode()' is being called on it. This will fail since BytesIO objects don't have an encode method. Use 'content.getvalue()' to get the bytes content, or ensure content is properly handled as bytes.
Copilot uses AI. Check for mistakes.
) | ||
write_with_timeout( | ||
process.stdin, | ||
content.encode(), |
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.
Same issue as above - 'content' is a BytesIO object and doesn't have an encode() method. Use 'content.getvalue()' instead.
Copilot uses AI. Check for mistakes.
b"Server: %(server)s\r\n" | ||
b"\r\n" | ||
) % { | ||
b'length': len(content.encode()), |
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.
Same issue - 'content' is a BytesIO object and doesn't have an encode() method. Use 'content.getvalue()' instead.
Copilot uses AI. Check for mistakes.
) | ||
await write_with_timeout( | ||
process.stdin, | ||
content.encode(), |
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.
Same issue - 'content' is a BytesIO object and doesn't have an encode() method. Use 'content.getvalue()' instead.
Copilot uses AI. Check for mistakes.
2dae8f7
to
8dbf492
Compare
8dbf492
to
3bc9436
Compare
No description provided.