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

CloudEvent must be an abstraction of the spec, and not JSON specific #12959

Closed
lmazuel opened this issue Aug 7, 2020 · 1 comment · Fixed by #13542
Closed

CloudEvent must be an abstraction of the spec, and not JSON specific #12959

lmazuel opened this issue Aug 7, 2020 · 1 comment · Fixed by #13542
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Event Grid Messaging Messaging crew

Comments

@lmazuel
Copy link
Member

lmazuel commented Aug 7, 2020

Again, I'm not saying the current prototype is incorrect, but writting this issue so I'm sure we address the spec explicitly.

Cloud Event is an abstraction of what is an event: https://github.com/cloudevents/spec/blob/v1.0/spec.md

As so, our CloudEvent class should not assume any serialization of a CloudEvent concept, and model the concept without thinking about the serialization. This implies:

  • There should be no data_base64 attribute, but helpers to set bytes correctly
  • Extensions should be treated as explicit extensions (and not simple new attributes at the base level).

Example:

c = CloudEvent(
    id=12,
    source='MyApp',
    type='CreateStuff'
)  # specversion is set by default to "1.0"

c.subject = "mynewfile.jpg"  # I could have set it as kwargs, but for the sake of sample

# Set data. API here could be discuss, we could use `data` property setter for instance
c.set_bytes_data(b"PDF%...", "application/pdf")

# Extensions are allowed aside, to avoid collision
c.extensions["reasonCode"] = 204

c.model # Returns the deserialized model

A JSON serialization of this would be:

{
    "id": 12,
    "source": "MyApp",
    "type": "CreateStuff",
    "subject": "mynewfile.jpg",
    "datacontenttype": "application/pdf",
    "data_base64": "ABCDEF_BASE64_OF_PDF",
    "reasonCode": 204
}

While a binary mode serialization would be HTTP request

ce-id: 12
ce-source
ce-source: MyApp
ce-type: CreateStuff
ce-subject: mynewfile.jpg
ce-datacontenttype: application/pdf
ce-reasonCode: 204

PDF%...
@lmazuel lmazuel added Event Grid Client This issue points to a problem in the data-plane of the library. labels Aug 7, 2020
@lmazuel lmazuel added this to the [2020] September milestone Aug 7, 2020
@KieranBrantnerMagee
Copy link
Member

Notes from our OOB discussion:

Extensions:

  • take as kwargs, potential risk overlap with data_base64/data args.

Binary vs. json serialization:

  • kwarg overload in send() to let control of this. We assume b64 in absence of any guidance since it "will just work" consistently.
    • Have enum for overload type, e.g. JSON or binary serialization mode.
  • won't need set_bytes_data since this will be handled at send time.
  • data as string? send plain. bytes? b64 encode and send. (with optional overload.)
    • this is gonna be wacky in 2.7. Perhaps "if unicode send directly; if otherwise encode and bytes."

c.model: StorageBlobDeletedEvent, keep in mind this may shift as we adjust the "GetData" discussions.

@lmazuel lmazuel added the Messaging Messaging crew label Apr 12, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. Event Grid Messaging Messaging crew
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants