Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions python/rnet/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,11 @@ class Client:
asyncio.run(main())
```
"""

async def __aenter__(self) -> Any: ...
async def __aexit__(
self, _exc_type: Any, _exc_value: Any, _traceback: Any
) -> Any: ...
Comment on lines +1358 to +1361
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The type hints for __aenter__ and __aexit__ can be more specific. __aenter__ should return an instance of the class, so its return type should be "Client". The __aexit__ method in the Rust implementation doesn't return a value, which corresponds to None in Python.

Suggested change
async def __aenter__(self) -> Any: ...
async def __aexit__(
self, _exc_type: Any, _exc_value: Any, _traceback: Any
) -> Any: ...
async def __aenter__(self) -> "Client": ...
async def __aexit__(
self, _exc_type: Any, _exc_value: Any, _traceback: Any
) -> None: ...


async def delete(
url: str,
Expand Down
7 changes: 5 additions & 2 deletions python/rnet/blocking.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def close(self) -> None:
in future versions.
"""

def __enter__(self) -> "Response": ...
def __enter__(self) -> Any: ...
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The return type of __enter__ was changed from "Response" to Any. This is a regression in type hint specificity. The __enter__ method should return self, so the type hint should be "Response". Please revert this change.

Suggested change
def __enter__(self) -> Any: ...
def __enter__(self) -> "Response": ...

def __exit__(self, _exc_type: Any, _exc_value: Any, _traceback: Any) -> None: ...
def __str__(self) -> str: ...

Expand Down Expand Up @@ -204,7 +204,7 @@ def close(
* `reason` - An optional reason for closing.
"""

def __enter__(self) -> "WebSocket": ...
def __enter__(self) -> Any: ...
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The return type of __enter__ was changed from "WebSocket" to Any. This is a regression in type hint specificity. The __enter__ method should return self, so the type hint should be "WebSocket". Please revert this change.

Suggested change
def __enter__(self) -> Any: ...
def __enter__(self) -> "WebSocket": ...

def __exit__(self, _exc_type: Any, _exc_value: Any, _traceback: Any) -> None: ...
def __str__(self) -> str: ...

Expand Down Expand Up @@ -463,6 +463,9 @@ def get(
```
"""
...

def __enter__(self) -> Any: ...
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The __enter__ method should return an instance of the class itself. The type hint should be "Client" instead of Any for consistency with other context managers in the codebase.

Suggested change
def __enter__(self) -> Any: ...
def __enter__(self) -> "Client": ...

def __exit__(self, _exc_type: Any, _exc_value: Any, _traceback: Any) -> None: ...


def delete(
Expand Down
34 changes: 34 additions & 0 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,21 @@ impl Client {
}
}

#[pymethods]
impl Client {
#[inline]
async fn __aenter__(slf: Py<Self>) -> PyResult<Py<Self>> {
Ok(slf)
}

#[inline]
async fn __aexit__(&self, _exc_type: Py<PyAny>, _exc_val: Py<PyAny>, _traceback: Py<PyAny>) {
// TODO: Implement connection closing logic if necessary.
}
Comment on lines +596 to +598
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The __aexit__ method is currently a placeholder. For a Client context manager to be useful, it should handle resource cleanup, such as closing connections. Please consider implementing the necessary cleanup logic here to fulfill the contract of a context manager.

}
Comment on lines +588 to +599
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For better code organization and readability, it's a good practice to consolidate all #[pymethods] for a single struct into one impl block. Consider merging this impl Client block with the one starting on line 250.


// ===== impl BlockingClient =====

#[pymethods]
impl BlockingClient {
/// Creates a new blocking Client instance.
Expand Down Expand Up @@ -729,3 +744,22 @@ impl BlockingClient {
})
}
}

#[pymethods]
impl BlockingClient {
#[inline]
fn __enter__(slf: PyRef<Self>) -> PyRef<Self> {
slf
}

#[inline]
fn __exit__<'py>(
&self,
_py: Python<'py>,
_exc_type: &Bound<'py, PyAny>,
_exc_value: &Bound<'py, PyAny>,
_traceback: &Bound<'py, PyAny>,
) {
// TODO: Implement connection closing logic if necessary.
}
Comment on lines +756 to +764
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Similar to the Client, the __exit__ method for BlockingClient is a placeholder. To make the context manager useful, this method should implement resource cleanup logic, such as closing any open connections managed by the client.

}
Comment on lines +748 to +765
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For better code organization and readability, it's a good practice to consolidate all #[pymethods] for a single struct into one impl block. Consider merging this impl BlockingClient block with the one starting on line 603.

Loading