JSONRPC Async Model #331
JSONRPC Async Model #331
Conversation
|
Got a question. if (result.StartsWith("DEBUG:"))
{
MessageBox.Show(new Form {TopMost = true}, result.Substring(6));
return string.Empty;
}Does anyone actually use this to debug? This prevents making it based on Stream, because Stream are impossible to peek. I think it will be better to parse it into another field in |
|
Does this just show a message box when result starts with debug? |
Yes |
|
It will prevent from assuming the returning message is based on JsonRPCQueryResponseModel so that we can't deserialize it based on stream. |
|
Yeah let's get rid of it. Surely there is a better implementation for debugging |
|
Ready for review? |
|
Ah yes! |
|
The to dos in the PR description are no longer relevant right? |
Yeah they are questions, not quite relate to the change in this branch. |
|
Have we done any testing with this? |
Hmmm i havent played enough with non-c# plugins to be able to answer those. General rule of thumb, if it improves Flow and you have tested it to make sure, then lets PR it. Leaving those questions here I will likely forget, unless you will remember :) |
I check out that python plugin will work if python works. If It doesn't work, the error reporting part will be triggered (I think the EndofStream works). |
| public abstract string SupportedLanguage { get; set; } | ||
|
|
||
| protected abstract string ExecuteQuery(Query query); | ||
| protected abstract Task<Stream> ExecuteQueryAsync(Query query, CancellationToken token); |
There was a problem hiding this comment.
Is this change backwards compatible?
There was a problem hiding this comment.
The JSONRPCPlugin is in Flow, and what it is doing is that overriding the Query (or QueryAsync) to call the executable (or python) and communicate via console stream. So change here won't affect the compatibility of plugins.
| } | ||
| } | ||
|
|
||
| public async Task<List<Result>> QueryAsync(Query query, CancellationToken token) |
There was a problem hiding this comment.
the change from Query to QueryAsync, will this be backwards compatible for plugins using JsonRPC?
Well actually these questions are also relate to c# plugins. What do you think about them? |
|
The change in this branch are majorly using the IAsyncPlugin to allow early stop (not completely because I don't know whether we can stop the external program), and read Stream instead of reading string to reduce the cost of creating string based on Standard output stream. |
oh yeah you are right. I think why not, we can do it for QueryAsync implementation and test it, not that it's a necessity right now though, but implementing it may allow for future usage and expansion. |
|
Let's create issues for the questions and can discuss or implement it. |
2 participants
Question: