✨ Migrated JSON RPC server to use asyncio and added cancellation support #624
Conversation
JonahSussman
requested review from
shawn-hurley,
fabianvf and
pranavgaikwad
and removed request for
shawn-hurley
|
Closing for now, will re-open once we determine it's time to move to asyncio. |
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
JonahSussman
force-pushed
the
feature/asyncio
branch
from
bd19c4a to
8ad8666
Compare
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
JonahSussman
changed the title
| self.args.append("-depOpenSourceLabelsFile") | ||
| self.args.append(str(dep_open_source_labels_path)) | ||
|
|
||
| self.excluded_paths = excluded_paths |
There was a problem hiding this comment.
what is going on with excluded_paths here?
There was a problem hiding this comment.
I do believe that we are setting the defaults in __init__
| # StreamReader's limit, it will just straight up not repopulate the | ||
| # buffer with fresh data. Thus, we need to read in chunks. | ||
| left = content_length | ||
| chunk_size = 1024 |
There was a problem hiding this comment.
isnt limit something like 64KiB? 1024 sounds too low
There was a problem hiding this comment.
Experimentally, I was having consistency issues with anything higher than 1024
| if self.recv_file.closed: | ||
| while True: | ||
| try: | ||
| result += await reader.readuntil(b"\n") |
There was a problem hiding this comment.
wouldnt this reader have the same problem as above?
There was a problem hiding this comment.
No because once a stream is exhausted, the StreamReader will reliably return EOF https://docs.python.org/3/library/asyncio-stream.html#asyncio.StreamReader
Unfortunately, this PR is pretty large because it has to be.
Why move to
asyncio? We could only handle processing one incoming request or notification at a time. This was undesirable for many reasons, one of them being bi-directional communication with the IDE. Now we can! Take a look attests/test_jsonrpc.pyfor an example of it succeeding. Hopefully the server can do more useful stuff while waiting around for the LLM calls.Also added a
getCodeplanAgentSolution.cancelto cancel current requestTodos:
getCodeplanAgentSolutionin one big lock.