Introduce driver.execute_query #833
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bigmontz
approved these changes
Oct 19, 2022
robsdedude
commented
Oct 27, 2022
docs/source/api.rst
Outdated
| :returns: the result of the ``result_transformer`` | ||
| :rtype: T | ||
|
|
||
| .. versionadded:: 5.2 |
There was a problem hiding this comment.
docs/source/api.rst
Outdated
| :members: session, query_bookmark_manager, encrypted, close, | ||
| verify_connectivity, get_server_info | ||
|
|
||
| .. method:: execute_query(query, parameters=None,routing=neo4j.RoutingControl.WRITERS, database=None, impersonated_user=None, bookmark_manager=self.query_bookmark_manager, result_transformer=Result.to_eager_result, **kwargs) |
There was a problem hiding this comment.
Can we have a :raises: field? Or does it raise too many different exceptions?
There was a problem hiding this comment.
There are plenty of errors this can raise. Here are the one's I can think of on the spot
Neo4jError(or any sub-class) - anything the server complains aboutSessionExpired,ServiceUnavailable(or any sub-class) - lost/couldn't establish connectivity (after the retries this API includes)ConfigurationErrorTypeErrorValueError(maybe evenIndexError?) - invalid configuration/parameters or trying to use a feature that's too new for the server you're connected toBoltError(or any sub-class; internal error class you shouldn't ever encounter) - the server violated the protocol
There might be more that I can't think of right now. My conclusion:
- All of there errors are fatal (there is pretty much no benefit in giving them different treatment). The user might as well just catch any exception, maybe log it or whatever they feel doing, and then give up (that could mean crash, surface some error to the end-user, whatever is suitable). Plus maybe fixing their code afterwards.
- I can hardly give any guarantees what errors this method can raise as it pretty much exercises the whole stack of the driver. It's highly likely that I'll forget some corner of the driver and, even if not, that this piece of documentation will unsync sooner or later as it's so far up the abstraction stack.
So much for why I didn't include it. If you think it'd be helpful nonetheless, we can make it happen.
There was a problem hiding this comment.
The user might as well just catch any exception
👍
robsdedude
force-pushed
the
driver-level-api
branch
from
a2b80f1 to
7ff6803
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depending on
Driver.execute_queryneo4j-drivers/testkit#531