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

Immudb 1.3.2 with backward compability fix #53

Merged
merged 4 commits into from
Sep 1, 2022
Merged

Conversation

Razikus
Copy link
Collaborator

@Razikus Razikus commented Aug 31, 2022

No description provided.

@Razikus Razikus requested a review from mabmayer August 31, 2022 15:39
@@ -26,7 +26,7 @@ def call(service: schema_pb2_grpc.ImmuServiceStub, rs: RootService, key: bytes,
try:
msg = service.Get(request)
except Exception as e:
if hasattr(e, 'details') and e.details() == 'key not found':
if hasattr(e, 'details') and (e.details() == 'key not found' or e.details() == 'tbtree: key not found'):

Choose a reason for hiding this comment

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

I'd suggest checking for the substring key not found. Next immudb release will expose error codes, making error handling easier and less error prone

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can check with endswith() method
If i would simply check for if string is in string then we would have another backward compability issue - with expiration.

Right now in immudb-py expiration returns error instead of None, so when i would check simply for existency of "key not found" in a string - it would start to returns None
if anyone right now are using this function and opaque it in try catch - try catch would stop working

This should be handled properly and consistent, but right now is not

Copy link
Collaborator Author

@Razikus Razikus Sep 1, 2022

Choose a reason for hiding this comment

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

@Razikus Razikus merged commit 48a8ebe into master Sep 1, 2022
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.

3 participants