Skip to content
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

Update RpcClient::invoke_method() to return a UMessage in support of symmetry / flexibility #27

Conversation

PLeVasseur
Copy link
Contributor

Hey there 👋

As I worked on the uStreamer we figured out we'll need RpcClient::invoke_method() to return a UMessage in order to make the operations performed by the uStreamer upon the CEs coming in and out as generic as possible.

@stevenhartley put out a spec change and made the change over in up-java already. More details can be understood from there.

@PLeVasseur PLeVasseur changed the title Update RpcClient::invoke_method() to return a UMessage in support of pluggable UStreamer Update RpcClient::invoke_method() to return a UMessage in support of symmetry / flexibility Jan 30, 2024
@PLeVasseur
Copy link
Contributor Author

PLeVasseur commented Jan 30, 2024

Amending this -- discussion with @stevenhartley came to the conclusion that we should use uP-L1 send() and register_listener() only for uStreamer operation to make us able to have a more generic implementation.

However, returning a UMessage from uP-L2's invoke_method does give some symmetry and flexibility to the API.

Looking to hear what folks have to think

Copy link

@stevenhartley stevenhartley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dakrbo & @sophokles73 let me know your thoughts, +1 from me

Copy link
Contributor

@sophokles73 sophokles73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PLeVasseur
Copy link
Contributor Author

Closing this one in favor of @AnotherDaniel's PR over here which does some more general clean up as well

@PLeVasseur PLeVasseur closed this Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants