-
Notifications
You must be signed in to change notification settings - Fork 591
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
Avro Event Format for CloudEvents #438
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fabiojose, a few first remarks inline.
avro-format.md
Outdated
|
||
### 2.2 Mapping Any-typed Attributes | ||
|
||
`Any`-typed CloudEvents values can either hold a `String`, or a `Binary` value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be all the other types.
avro-format.md
Outdated
### 2.2 Mapping Any-typed Attributes | ||
|
||
`Any`-typed CloudEvents values can either hold a `String`, or a `Binary` value, | ||
or a `Map`. Avro type system SHOULD NOT satisfy the map-with-maps in a recursive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the motivation for discouraging maps of maps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gunnarmorling It's possible, but the map inside of another map needs the type declaration. It is different from JSON maps.
Let's see an example:
{
"name":"metadata",
"type":{
"type":"map",
"values":[
"null",
"int",
"float",
"string",
"boolean",
"long",
{
"type":"map",
"values":[
"null",
"int",
"float",
"string",
"boolean",
"long"
]
}
]
}
}
If I want another map
I need to declare. There is no way to declare a Any
-typed map.
avro-format.md
Outdated
"type":"record", | ||
"name":"CloudEvent", | ||
"version":"0.2", | ||
"doc":"Avro Event Format fo CloudEvents", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/fo/of/
@fabiojose can you sign your commits? |
avro-format.md
Outdated
|
||
## Abstract | ||
|
||
The Avro Format for CloudEvents defines how events attributes are expressed in the [Avro 1.9.0 Specification][avro-spec]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please wrap things at 80 columns
avro-format.md
Outdated
| String | [string][avro-primitives] | | ||
| Binary | [bytes][avro-primitives] | | ||
| Map | [map][avro-primitives] | | ||
| Any | See [2.2](#22-mapping-any-typed-attributes) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that the order is the same as what's in the spec... but ANY has been moved to the end now :-)
avro-format.md
Outdated
|
||
CloudEvents Spec defines optional attributes. The Avro format defines that | ||
every optional field MUST use the `null` type and the actual type through | ||
the [union][avro-unions]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that optional attributes will always appear, just with a "null" value? Is it a "MUST appear" or "MAY appear"? What happens if the send doesn't include it - are they violating something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@duglin it is "MAY appear". Using the null
means that an optional attribute appears if it has value or no appear if there is no value at all.
avro-format.md
Outdated
"namespace":"io.cloudevents", | ||
"type":"record", | ||
"name":"CloudEvent", | ||
"version":"0.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0.3
@fabiojose can you address the comments before Tuesday EOD? |
avro-format.md
Outdated
@@ -0,0 +1,176 @@ | |||
# Avro Event Format for CloudEvents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, please add - Version 0.4-wip
to the tile.
@fabiojose, one of the checks failed (commit signoffs). See details: https://github.com/cloudevents/spec/pull/438/checks?check_run_id=145188904 |
@fabiojose you around? |
This PR closes #426.
This is the first version, please comment and improve!