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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the Surreal.patch method #100

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Wouterkoorn
Copy link

Change request method type from "modify" to "patch". And fix type annotations.

What is the motivation?

Surreal.patch is currently not usable.
Fixing type annotations improved developer experience.

Type of Change

  • 馃摎 Examples / docs / tutorials / dependencies update
  • 馃敡 Bug fix (non-breaking change which fixes an issue)
  • 馃 Improvement (non-breaking change which improves an existing feature)
  • 馃殌 New feature (non-breaking change which adds functionality)
  • 馃挜 Breaking change (fix or feature that would cause existing functionality to change)
  • 馃攼 Security fix

What does this change do?

  1. Changes the Surreal.patch request method type from "modify" to "patch"
  2. Changes the Surreal.patch's data parameter type to Optional[List[Dict[str, Any]]]
  3. Changes the Surreal.patch return type to Union[dict[str, Any], List[Dict[str, Any]]]

What is your testing strategy?

mypy OK
pre-commit OK
./scripts/run_tests.sh OK
manual testing OK

Is this related to any issues?

Yes: #99

Have you read the Contributing Guidelines?

Change request method type from "modify" to "patch". And fix type annotations.
@@ -598,6 +600,7 @@ async def patch(
{ 'op': "remove", "path": "/temp" },
])
"""
data = data or []
Copy link
Author

@Wouterkoorn Wouterkoorn Apr 2, 2024

Choose a reason for hiding this comment

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

This is for backwards compatibility, making the change non-breaking.
The typing was Optional, but the server requires an array, None is invalid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant