Skip to content
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

Bug: Refactor HTTP client. #59

Open
2 tasks done
Ce11an opened this issue Apr 22, 2023 · 6 comments
Open
2 tasks done

Bug: Refactor HTTP client. #59

Ce11an opened this issue Apr 22, 2023 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@Ce11an
Copy link
Contributor

Ce11an commented Apr 22, 2023

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

[email protected]

Is there an existing issue for this?

  • I have searched the existing issues

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Ce11an Ce11an added the bug Something isn't working label Apr 22, 2023
@AlexFrid AlexFrid self-assigned this Apr 24, 2023
@AlexFrid
Copy link
Contributor

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
Copy link
Contributor Author

Ce11an commented Apr 24, 2023

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
Copy link
Contributor

Yeah, absolutely

Ce11an added a commit to Ce11an/surrealdb.py that referenced this issue Apr 24, 2023
# type: ignore had to be added to the HTTP client. The client needs to be refactored, see: surrealdb#59
@Ce11an Ce11an mentioned this issue Apr 24, 2023
2 tasks
@PythonTryHard
Copy link

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

@Ce11an
Copy link
Contributor Author

Ce11an commented Apr 25, 2023

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants