Codex/set up mkdocs documentation structure#107
Conversation
WalkthroughUpdated type annotations and method signatures in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pysrc/riffq/connection.py (1)
149-149: Add spaces around colons in type annotations.The type hints are missing spaces after the colons, which is inconsistent with PEP 8 style guidelines.
Apply this diff:
- def set_tls(self, crt:str, key:str): + def set_tls(self, crt: str, key: str):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pysrc/riffq/connection.py(16 hunks)pysrc/riffq/helpers.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pysrc/riffq/connection.py (2)
src/lib.rs (3)
set_tls(1677-1685)register_table(1695-1729)start(1733-1737)tests/test_connection_id.py (1)
handle_query(19-30)
🪛 GitHub Actions: CI
pysrc/riffq/helpers.py
[error] 21-21: AttributeError: module 'pyarrow' has no attribute '_ffi' when defining to_arrow(schema_desc: list[dict], rows: list) -> pa._ffi.lib.PyCapsule.
🪛 Ruff (0.14.0)
pysrc/riffq/connection.py
80-80: Unused lambda argument: a
(ARG005)
80-80: Unused lambda argument: k
(ARG005)
92-92: Unused method argument: ip
(ARG002)
92-92: Unused method argument: port
(ARG002)
92-92: Unused lambda argument: a
(ARG005)
92-92: Unused lambda argument: k
(ARG005)
103-103: Unused lambda argument: a
(ARG005)
103-103: Unused lambda argument: k
(ARG005)
117-117: Unused method argument: ip
(ARG002)
117-117: Unused method argument: port
(ARG002)
117-117: Unused lambda argument: a
(ARG005)
117-117: Unused lambda argument: k
(ARG005)
212-212: Unused lambda argument: a
(ARG005)
212-212: Unused lambda argument: k
(ARG005)
227-227: Unused lambda argument: a
(ARG005)
227-227: Unused lambda argument: k
(ARG005)
253-253: Unused lambda argument: a
(ARG005)
253-253: Unused lambda argument: k
(ARG005)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: unit-tests
🔇 Additional comments (4)
pysrc/riffq/connection.py (4)
1-15: LGTM: Clear module-level documentation.The updated module docstring provides a helpful overview of the two main concepts (
BaseConnectionandRiffqServer) and explains the callback pattern used throughout.
41-41: LGTM: Type annotations improve BaseConnection API clarity.The added type hints across all
BaseConnectionmethods make the interface contract explicit and will help with IDE autocomplete and type checking. The parameter and return types are accurate.Also applies to: 45-45, 64-64, 80-80, 92-92, 103-103, 117-117
136-147: LGTM: Type annotations improve RiffqServer initialization.The type hints on
__init__and instance variables clarify the expected types. The explicitDict[int, BaseConnection]andType[BaseConnection]annotations are particularly helpful for understanding the connection management pattern.
187-202: LGTM: Type annotations and docstring improvements.The remaining method signatures now have clear type hints, and the docstrings have been updated to better describe parameters and behavior. The changes particularly improve clarity around:
register_tablecolumn descriptor format- Connection ID handling across forwarding methods
- TLS and catalog emulation options in
startAlso applies to: 204-210, 212-225, 227-238, 240-251, 253-268, 270-280
|
|
||
| def to_arrow(schema_desc, rows): | ||
| """Build an Arrow C Stream from schema and rows. | ||
| def to_arrow(schema_desc:list[dict], rows:list) -> pa._ffi.lib.PyCapsule: |
There was a problem hiding this comment.
Fix the invalid return type annotation.
The return type pa._ffi.lib.PyCapsule is invalid and causes the pipeline failure. PyArrow does not expose the _ffi.lib.PyCapsule path. The __arrow_c_stream__() method returns a PyCapsule object from Python's C API, which cannot be directly referenced as a type in this manner.
Apply this diff to use a valid type annotation:
-def to_arrow(schema_desc:list[dict], rows:list) -> pa._ffi.lib.PyCapsule:
+def to_arrow(schema_desc: list[dict], rows: list) -> Any:Note: This also adds the missing spaces around : for PEP 8 compliance.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: CI
[error] 21-21: AttributeError: module 'pyarrow' has no attribute '_ffi' when defining to_arrow(schema_desc: list[dict], rows: list) -> pa._ffi.lib.PyCapsule.
🤖 Prompt for AI Agents
In pysrc/riffq/helpers.py around line 21, the return type annotation
pa._ffi.lib.PyCapsule is invalid; change the signature to use a valid type
(e.g., from typing import Any and annotate the return as -> Any) and ensure
there is a space after the parameter names and before the type hints
(schema_desc: list[dict], rows: list) to satisfy PEP 8; also add the necessary
import for Any at the top of the file if not present.
| Example usage: | ||
| >>> callback(to_arrow([{"name": "val", "type": "int"}], [ | ||
| [1], | ||
| [2] | ||
| ])) | ||
|
|
There was a problem hiding this comment.
Fix the example indentation in the docstring.
The example code has incorrect indentation that makes it invalid Python syntax. List items should not be indented relative to the opening bracket.
Apply this diff to fix the indentation:
Example usage:
- >>> callback(to_arrow([{"name": "val", "type": "int"}], [
- [1],
- [2]
- ]))
+ >>> callback(to_arrow([{"name": "val", "type": "int"}], [[1], [2]]))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Example usage: | |
| >>> callback(to_arrow([{"name": "val", "type": "int"}], [ | |
| [1], | |
| [2] | |
| ])) | |
| Example usage: | |
| >>> callback(to_arrow([{"name": "val", "type": "int"}], [[1], [2]])) |
🤖 Prompt for AI Agents
In pysrc/riffq/helpers.py around lines 29 to 34, the docstring example has
mis-indented list items inside the bracket which makes the snippet invalid
Python; fix it by left-aligning the inner list items so they are not indented
relative to the opening bracket (i.e., remove the extra spaces before [1] and
[2]) so the example reads as a valid call to callback(to_arrow(..., [[1],
[2]])).
1 participant
Summary by CodeRabbit
Documentation
Refactor