-
Notifications
You must be signed in to change notification settings - Fork 1
Async Rpc Server Implementation #1
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Cristian <[email protected]>
Signed-off-by: Cristian <[email protected]>
Signed-off-by: Cristian <[email protected]>
.vscode/settings.json
Outdated
| @@ -0,0 +1,5 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should ignore .vscode/*
generated/Async_rpc.Bin_prot_generated_types.Lib.Async_rpc_kernel.Src.Protocol.fs
Show resolved
Hide resolved
src/Implementation.fs
Outdated
| -> pos_ref | ||
| -> Transport.Writer.t | ||
| -> Result.t<unit, Rpc_error.t> | ||
| <<<<<<< HEAD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few merge conflicts here that we should resolve
src/Implementation.fs
Outdated
| >>>>>>> ec23e7a927aa331442e8fa83058b083345c54c1d | ||
| } | ||
|
|
||
| let real_function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of [real_function] we should give it a name that explains what is happening
src/Implementation.fs
Outdated
|
|
||
| match result with | ||
| | Ok () -> () | ||
| | Error _error -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of ignoring all of these results could we print them out so that theres some sort of logging on the server-side? Also we should add a comment about why these are being ignored (since they only fail when there's an issue with the connection in which case the connection will close automatically)
src/Server.fs
Outdated
| while true do | ||
| Console.WriteLine "Waiting for connection..." | ||
|
|
||
| task { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are some parts task and others async? Is this related to the task in nunit test but that we saw yesterday?
src/Transport.fsi
Outdated
| val close: t -> unit | ||
| val is_close_started: t -> bool | ||
| val set_close_finished_callback: t -> (Close_reason.t -> unit) -> unit | ||
| (*TODO is this the correct function to write the binary protocol?*) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra todo
test/.vscode/settings.json
Outdated
| @@ -0,0 +1,5 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding **/.vscode to the gitignore would also be good
test/Async_rpc.Test.fsproj
Outdated
| <PackageReference Include="nunit" Version="3.13.2" /> | ||
| <PackageReference Include="NUnit3TestAdapter" Version="4.0.0" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.0.0" /> | ||
| <!--<PackageReference Include="xunit" Version="2.4.2" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can delete these now that we fixed the nunit confusion
| (bin_reader_t (bin_a : _ Bin_prot.Type_class.t).reader) | ||
| } | ||
| end | ||
| (*TODO*) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra todos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 1 todo here
| ()) | ||
|
|
||
| task { | ||
| Async_rpc.Connection.create |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is already wrapped in a task internally so we might not need this one?
|
|
||
| ()) | ||
|
|
||
| let implementation_f = (fun _foo _bar -> ()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we specifying a implementation for the client?
| let start_server () = | ||
| let time = new Time_source.Wall_clock.t () | ||
|
|
||
| let implementation_fun = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets use a more descriptive name - like wait_forever or sometihng
|
|
||
| dispatch_tcs | ||
|
|
||
| let start_client port = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start_client_and_dispatch_query?
|
|
||
| async { | ||
| let resp = | ||
| //could the string here be anything and it would work? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra comment
| (bin_reader_t (bin_a : _ Bin_prot.Type_class.t).reader) | ||
| } | ||
| end | ||
| (*TODO*) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still 1 todo here
Async Rpc Server Implementation