-
Notifications
You must be signed in to change notification settings - Fork 207
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
rpc: Support handlers of any arity #4630
base: main
Are you sure you want to change the base?
Conversation
@@ -28,7 +28,7 @@ func newAspireService(server *Server) *aspireService { | |||
// GetAspireHostAsync is the server implementation of: | |||
// ValueTask<AspireHost> GetAspireHostAsync(Session session, string aspireEnv, CancellationToken cancellationToken). | |||
func (s *aspireService) GetAspireHostAsync( | |||
ctx context.Context, rc RequestContext, aspireEnv string, observer IObserver[ProgressMessage], | |||
ctx context.Context, rc RequestContext, aspireEnv string, observer *IObserver[ProgressMessage], |
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.
It's non-obvious from the diff why I went and did this. The issues that IObserver needs some special attention during unmarshalling. The way that works is we have an internal interface:
// connectionObserver is an interface that can be implemented by types that want to be notified when they are deserialized
// during a JSON RPC call.
type connectionObserver interface {
attachConnection(c jsonrpc2.Conn)
}
When we unmarshall arguments, we check to see if the type we unmarshalled to implements this interface and if so, we call it. IObserver's implementation of the method stores the connection so it can send messages to it later, as items are emitted.
The interface is implemented on *IObserver[T]
not IObserver[T]
.
The old version of unmarshalArg
handled this by adding a level of indirection on thet ype during interface checking:
if v, ok := (any(&arg)).(connectionObserver); ok {
v.attachConnection(conn)
}
But it felt cleaner to not do this extra level of indirection in the new version and instead ensure the argument was just *IObserver[T]
.
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 can image that we could also do both - check to see if T and *T implement the interface, (preferring one if both are there) and calling the function.
40e6c3f
to
26c4363
Compare
26c4363
to
92112c9
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
No description provided.