Bug: Refactor HTTP client.

Ce11an commented 1 year ago

Describe the bug

The HTTP client needs refactoring. There are several issues that I have found when running mypy.

Some of these errors include:

surrealdb/http.py:102: error: "SurrealHTTP" has no attribute "disconnect"; maybe "connect"?  [attr-defined]
surrealdb/http.py:147: error: Incompatible return value type (got "SurrealResponse", expected "str")  [return-value]
surrealdb/http.py:159: error: Incompatible return value type (got "SurrealResponse", expected "str")  [return-value]
surrealdb/http.py:184: error: Incompatible return value type (got "SurrealResponse", expected "List[Dict[str, Any]]")  [return-value]
surrealdb/http.py:213: error: Value of type "SurrealResponse" is not indexable  [index]
surrealdb/http.py:246: error: Value of type "SurrealResponse" is not indexable  [index]
surrealdb/http.py:281: error: Value of type "SurrealResponse" is not indexable  [index]
surrealdb/http.py:314: error: Value of type "SurrealResponse" is not indexable  [index]
surrealdb/http.py:336: error: No return value expected  [return-value]

As well as this there are several differences between the HTTP client and WS client. This can be confusing to users. For example:

The __init__ method takes different arguments.
Several methods are missing in the HTTP client.

Steps to reproduce

If mypy is installed in your environment:

poetry run mypy .

Expected behaviour

No errors when running mypy.
The methods and between clients are the same.

SurrealDB version

surreal 1.0.0-beta.8+20220930.c246533 for macos on x86_64

surrealdb.py version

surrealdb.py 0.30 for macOS on aarch64 using Python 3.9.1

Contact Details

hallcellan@gmail.com

Is there an existing issue for this?

[X] I have searched the existing issues

Code of Conduct

[X] I agree to follow this project's Code of Conduct

AlexFrid commented 1 year ago

Yes, I totally agree that the HTTP client needs refactoring, and there is also plenty of functionality missing compared to the WS one.

This will be fixed in due course, but the HTTP implementation is not the highest priority at the moment, as we are first making sure the WS implementation for all major drivers is solid.

Ce11an commented 1 year ago

Yes, I totally agree that the HTTP client needs refactoring, and there is also plenty of functionality missing compared to the WS one.

This will be fixed in due course, but the HTTP implementation is not the highest priority at the moment, as we are first making sure the WS implementation for all major drivers is solid.

No problem. Would you be happy to implement features like mypy and unit tests for just the WS client (ignoring the HTTP client) for the time being?

AlexFrid commented 1 year ago

Yeah, absolutely

PythonTryHard commented 1 year ago

How much refactoring are we looking at here? I'd love to throw myself in the ring for once.

Ce11an commented 1 year ago

How much refactoring are we looking at here? I'd love to throw myself in the ring for once.

I think at the very least we would want the following done:

Fix the mypy errors.
Add any missing methods that are in the WS client in HTTP client.
Ensure that the methods take in the same arguments/types (if possible).

Let me know if I am missing anything @AlexFrid.

I am happy to help if needed 😄

AlexFrid commented 1 year ago

How much refactoring are we looking at here? I'd love to throw myself in the ring for once.

I think at the very least we would want the following done:

Fix the mypy errors.

Add any missing methods that are in the WS client in HTTP client.

Ensure that the methods take in the same arguments/types (if possible).

Let me know if I am missing anything @AlexFrid.

I am happy to help if needed 😄

That's pretty much it. Just bringing it to parity with the WS.

That being said, there might be cases where the HTTP API doesn't have some WS functionality built yet, as we are mainly focusing on WS at the moment, but it will come in due course. https://surrealdb.com/docs/integration/http

surrealdb / surrealdb.py