search

eclipse-uprotocol / up-spec

uProtocol Specifications
Apache License 2.0
32 stars 25 forks source link

Remove CallOptions from uattributes.proto #132

Closed gregmedd closed 4 months ago

gregmedd commented 4 months ago

The CallOptions message does not get included in any message or sent on the wire. It appears to be intended as a data wrapper for uE code interacting with the Layer 2 client API.

Since it is not a part of the actual message data model ("Layer 0"), it should be removed in favor of allowing language-specific APIs to implement the best fit for their language.

stevenhartley commented 4 months ago

Although your statement is true, I want to know why we couldn't define it here?

gregmedd commented 4 months ago

Because interacting with protobuf objects is really unpleasant in situations where it is not required. It is simpler to provide a context-specific interface that takes the appropriate parameters for that situation. See UMessageBuilder in up-rust. We are mirroring that API in the up-cpp updates.

Protobuf messages intended for use as purely internal APIs are only a hindrance to defining idiomatic language APIs. This goes back to the L2 update where the required methods are listed, but their signatures are left to the language-specific APIs to define.

sophokles73 commented 4 months ago

merged in https://github.com/eclipse-uprotocol/up-spec/pull/133

sophokles73 commented 4 months ago

Need to reopen because the spec has not been updated yet ...

stevenhartley commented 4 months ago

@sophokles73 you taking this or you want me to make the spec change?

sophokles73 commented 4 months ago

be my guest 😁

stevenhartley commented 4 months ago

@sophokles73 we are rewriting rpcclient.adoc anyways for the new high level L2 API specifications, shouldn't we do it at that point? The only reference to CallOptions are in that file.

stevenhartley commented 4 months ago

Closing, we will resolve reference in the L2 PR