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.

Sign up for GitHub

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

Jump to bottom

Definition/usage of message priority is ambiguous #71

Closed
sophokles73 opened this issue Feb 29, 2024 · 9 comments
Closed

Definition/usage of message priority is ambiguous #71

sophokles73 opened this issue Feb 29, 2024 · 9 comments
Assignees
@sophokles73
Labels
bug Something isn't working documentation Improvements or additions to documentation
Milestone
1.6.0-alpha.1

Comments

@sophokles73
Copy link
Contributor

Based on the definition of mapping common attributes to CloudEvents, my understanding is that request messages MUST contain a priority of at least CS4.

It is unclear, though, if response messages also need to contain a priority (of the same priority). This would make sense to me, because if I am interested in getting a response quickly (indicated by CS4 on the request), it seems reasonable to also require the response to be delivered high-prio, or am I mistaken?

Also, the default priority to assume when no priority is specified is defined inconsistently across the spec:

  • The source mentioned above defines the default to be CS0

  • The QoS specification defines the default value to be CS1, which also seems to be in line with the wording used in the UAttributes proto where CS1 is described as

    Standard, undifferentiated application such as General (unclassified)

FMPOV we need to adapt the CloudEvent attributes mapping section accordingly.
@sophokles73
Copy link
Contributor Author

@stevenhartley WDYT?

@sophokles73 sophokles73 mentioned this issue Feb 29, 2024
Merged
@stevenhartley
Copy link
Contributor

@sophokles73 Agree. Will you create a PR to fix this? Then we'll need to create PRs on the various language specs as well to ensure we choose CS1 as default and ensure CS4 for request and response (and they match).

@sophokles73 sophokles73 added bug Something isn't working documentation Improvements or additions to documentation labels Mar 11, 2024
@sophokles73
Copy link
Contributor Author

@stevenhartley I was wondering if it is still appropriate to define the general requirements regarding message attributes in the CloudEvents mapping spec. Most of the requirements stated in that document are actually independent from the way that a UMessage is being mapped to a transport.

We should probably move these requirements to the UAttributes spec or even the corresponding proto files? WDYT?

@PLeVasseur
Copy link
Contributor

We should probably move these requirements to the UAttributes spec or even the corresponding proto files? WDYT?

Agree. I was getting this feeling when I was reviewing the up-core-api change a week or so ago for priority to a uint32 and noticed that descriptions on how priority was handled and their defaults was not in some "general purpose" location.

@stevenhartley
Copy link
Contributor

We should probably move these requirements to the UAttributes spec or even the corresponding proto files? WDYT?

@sophokles73 agree. Lets document this in up-core-api uattribute.proto and ensure the client libraries implement this.

@sophokles73 sophokles73 mentioned this issue Mar 14, 2024
Merged
@sophokles73
Copy link
Contributor Author

I have tried to do that in eclipse-uprotocol/up-core-api#112.

However, now that I am trying to remove the corresponding information from the CloudEvents spec, I realize that the proto files are not a good place to define these general rules because there is only very limited support for documenting things, i.e. using C-type comments. Structuring the information is very difficult in proto and there is also no way of linking to certain message definitions from up-spec.

IMHO we should therefore better put the information in the textual documentation files in up-spec and use the proto files only as technical manifestation of the written spec.

Would that work for you @stevenhartley ?

@sophokles73
Copy link
Contributor Author

@PLeVasseur WDYT?

@PLeVasseur
Copy link
Contributor

I was also getting bummed out as I read more about Protobuf that we cannot put stronger constraints, but seems that's just how it goes.

IMHO we should therefore better put the information in the textual documentation files in up-spec and use the proto files only as technical manifestation of the written spec.

To me, this makes sense. Would we then put above each field / message in up-core-api a link back to the relevant docs in up-spec to avoid docs drifting from the .protos?

@sophokles73
Copy link
Contributor Author

@stevenhartley WDYT? Would you be okay with moving the info from the proto file(s) to properly formatted text documents in up-spec, e.g. uattributes.adoc?

@sophokles73 sophokles73 mentioned this issue Mar 21, 2024
Closed
@stevenhartley stevenhartley added this to the v1.0.0-alpha.1 milestone Apr 3, 2024
sophokles73 added a commit to SoftwareDefinedVehicle/uprotocol-spec-fork that referenced this issue Apr 10, 2024
@sophokles73
[eclipse-uprotocol#71] Remove ambiguity around uAttributes requirements
93e2608 
The requirements regarding the usage of particular UAttributes
properties for the message types supported by uProtocol had been
scattered around multiple documents, some textual descriptions as well
as proto files. It was hard to find the relevant information and to
keep it consistent across the specifications.

The following things have been done to mitigate the situation:
* The uattributes.adoc file is now the single source of truth regarding
  the usage of UAttributes for the supported message types. All
  corresponding definitions have been removed from the cloudevents.adoc
  and uattributes.proto files.
* The cloudevents.adoc file has been moved to the up-l1 folder because
  its content is relevant to uProtocol Client libraries only that use
  CloudEvents' JSON or Protobuf Format to map UMessages to a transport
  protocol's PDU. Application (uEntity) developers will not get in
  touch with CloudEvents at all.
@sophokles73 sophokles73 mentioned this issue Apr 10, 2024
Merged
sophokles73 added a commit to SoftwareDefinedVehicle/uprotocol-spec-fork that referenced this issue Apr 11, 2024
@sophokles73
[eclipse-uprotocol#71] Remove ambiguity around uAttributes requirements
3945b28 
The requirements regarding the usage of particular UAttributes
properties for the message types supported by uProtocol had been
scattered around multiple documents, some textual descriptions as well
as proto files. It was hard to find the relevant information and to
keep it consistent across the specifications.

The following things have been done to mitigate the situation:
* The uattributes.adoc file is now the single source of truth regarding
  the usage of UAttributes for the supported message types. All
  corresponding definitions have been removed from the cloudevents.adoc
  and uattributes.proto files.
* The cloudevents.adoc file has been moved to the up-l1 folder because
  its content is relevant to uProtocol Client libraries only that use
  CloudEvents' JSON or Protobuf Format to map UMessages to a transport
  protocol's PDU. Application (uEntity) developers will not get in
  touch with CloudEvents at all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sophokles73 sophokles73

Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Milestone
1.6.0-alpha.1
Development

No branches or pull requests
3 participants
@sophokles73 @stevenhartley @PLeVasseur