Add Redis Graph Commands #37
Conversation
shacharPash
requested review from
AviAvni and
slorello89
and removed request for
AviAvni
slorello89
left a comment
slorello89
left a comment
There was a problem hiding this comment.
The way that transactions work in here is really wonky.
StackExchange.Redis has this odd pattern where you:
- Create the transaction
- Execute "async" methods against the transaction
- Close the transaction
- pull the results from the tasks that were spawned by the async methods in step 2
this leads to a really weird issue where if you await a non-closing task before the closing task, you'll just deadlock your thread - which isn't great.
Here we have:
- Create the transaction
- Execute 'async' methods against the transaction
- close the transaction
- pull the results from an array of results that is returned when closing out the transaction.
In this model there isn't a great way to correlate the request to the result, and we've lied to the user with those async tasks as the tasks they got back have no deferred result which is really odd. Perhaps those methods shouldn't be called "Async" and shouldn't return any task, maybe "enqueue" or something with some way to correlate the command to the result.
| { | ||
| var results = new ResultSet[_pendingTasks.Count]; | ||
|
|
||
| var success = _transaction.Execute(); // TODO: Handle false (which means the transaction didn't succeed.) |
There was a problem hiding this comment.
if false this really should throw an aggregation of the error's that came out of the transaction.
| /// <summary> | ||
| /// Allows for executing a series of RedisGraph queries as a single unit. | ||
| /// </summary> | ||
| public class RedisGraphTransaction // TODO: check if needed |
shacharPash
requested review from
slorello89
and removed request for
leibale and
sazzad16
| @@ -0,0 +1,172 @@ | |||
| // using System.Text; | |||
|
LGTM 👍 |
No description provided.