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

clarification of links and inline content #6

Closed
tomkralidis opened this issue Aug 24, 2022 · 12 comments
Closed

clarification of links and inline content #6

tomkralidis opened this issue Aug 24, 2022 · 12 comments
Labels
question Further information is requested

Comments

@tomkralidis
Copy link
Collaborator

tomkralidis commented Aug 24, 2022

Should we consider the following

  • data SHALL be either encoded in a link object (with rel=canonical) OR inline in a content object via value

Rationale:

  • in some instances, the data notification message may actually be larger than the data payload
  • in some cases, it may take the broker/cache logic longer to download and put the data to an object in comparison to the actual message dissemination (related to real-time data)

We should also consider which user in a given publish sequence this could apply to (i.e. data provider, global services).

@david-i-berry feel free to update based on our discussion.

cc @golfvert

@tomkralidis tomkralidis added the question Further information is requested label Aug 24, 2022
@golfvert
Copy link
Contributor

I think the link is a MUST in all cases and inline content is a MAY for data below a certain size limit (eg 2kB). For consistency purposes, the link is a must for me. However, it is (probably) a matter of taste… more than anything else. All tastes are equally valid! (Tous les goûts sont dans la nature)

@kaiwirt
Copy link

kaiwirt commented Sep 9, 2022

We should not only take the size into account but also the type of the data. Warnings should be inline in my opinion.

@kaiwirt
Copy link

kaiwirt commented Sep 9, 2022

  • might also be useful for announcements or informational messages like out-of-service announcements

@tomkralidis
Copy link
Collaborator Author

Possible consideration:

  • for messages below 2048 bytes:
    • notifications SHALL provide inline (via properties.content)
    • notifications MAY additional provide via link object
  • for messages above 2048 bytes:
    • notifications SHALL provide via link object
    • notificaitions SHALL not provide inline

@golfvert
Copy link
Contributor

golfvert commented Dec 7, 2022

Just to confirm what was discussed... In fact, it should be:

for messages below 2048 bytes:

- notifications MAY provide inline (via properties.content)

- notifications SHALL additional provide via link object

for messages above 2048 bytes:

- notifications SHALL provide via link object

- notificaitions SHALL not provide inline

And the 2048 bytes limit is to be discussed!

@josusky
Copy link
Contributor

josusky commented Jan 12, 2023

In TT-Protocols we have converged on 4kB limit but the exact value can be re-considered after the pilot phase. Perhaps we could even allow a higher limit for high-priority things that are sent rarely, e.g. warnings. For sure the brokers shall be configured to handle notifications that are by an order of magnitude (or two) bigger - for resilience.
And we shall not forget that we have the possibility to gzip the embedded content. CAP and other XML-based messages are easily packed to 10% of their original size.

@amilan17
Copy link
Member

@josusky @tomkralidis is this still an open issue?

@tomkralidis
Copy link
Collaborator Author

We still need clear guidance; updated proposal:

Requirements:

  • notifications SHALL always provide a canonical link object (this is already in the specification, so no need to add per se)
  • for data whose resulting size (compressed or uncompressed) is greater than 4096 bytes, notifications SHALL NOT provide inline via properties.content.value

Permissions:

  • for data whose resulting size (compressed or uncompressed) is less than 4096 bytes, notifications MAY provide inline via properties.content.value

Thoughts @josusky @ktsuboi-jma @davidpodeur @McDonald-Ian @antje-s

@josusky
Copy link
Contributor

josusky commented May 22, 2023

In general, I agree.
I suppose that by

(compressed or uncompressed)

you mean that whatever is the intended content encoding, the inlined/embedded data shall not be greater. Perhaps we could clarify it in a separate sentence, like this:
"for data whose resulting size is greater than 4096 bytes, notifications SHALL NOT embed the data via properties.content.value. The limit takes into account the used data encoding, that is, if the data is compressed with gzip (properties.content.encoding has the value gzip), the compressed size matters.

And as stated earlier, we should consider an exception for specific types of data such as alerts/warnings (e.g. CAP).

@tomkralidis
Copy link
Collaborator Author

TT-WISMD 2023-05-22:

  • add @josusky comments to normative text/schema
  • need to review as part of pilot phase:
    • following pilot phase on 4096 byte
    • perhaps allow greater limits based on data types/frequency

@tomkralidis
Copy link
Collaborator Author

PR in #51

tomkralidis added a commit that referenced this issue May 23, 2023
* clarify content size and inline messages (#6)

* address PR comments
@antje-s
Copy link
Contributor

antje-s commented Aug 29, 2023

Close issue? PR #51 is already merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Development

No branches or pull requests

6 participants