-
Notifications
You must be signed in to change notification settings - Fork 577
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
feat: attrschema extension #1106
feat: attrschema extension #1106
Conversation
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Looks good to me. Thanks for all the work on this @sasha-tkachev ! |
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
$schema: "http://json-schema.org/draft-07/schema#" | ||
$id: "https://cloudevents.io/attrschema/2022-10/schema#" | ||
allOf: | ||
- { "$ref": "http://json-schema.org/draft-07/schema#" } |
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.
Looks like this is using draft-07 of JSON Schema. The current stable version is 2020-12. Is there a particular reason for not updating to 2020-12?
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.
no, fixing now
Signed-off-by: Alexander Tkachev <sasha64sasha@gmail.com>
- boolean | ||
- integer | ||
- string | ||
- binary |
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.
Is this saying that additional properties (are these for extensions?) can be either boolean, integer, string, or binary types?
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.
Yes,
Or URIs, references and timestamps
[Versioning of CloudEvents in the Primer](../primer.md#versioning-of-cloudevents) | ||
for more information. | ||
- Constraints: | ||
- OPTIONAL |
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.
REQUIRED
1. Create a map of all attributes of a given event, where the keys are the attribute | ||
names and the values are the attribute values | ||
2. Strip all `null` OPTIONAL attributes from the event if present | ||
3. All assumed values MUST be evaluated as-if the event was translated to another |
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'm not sure what "translated to another format" means. Can you elaborate?
Notice that this event is adhering to the example attrschema even though the | ||
`myattr` is of type `integer` and not `string`. The reason for it is | ||
that the `myattr` value MUST be converted to the string representation | ||
before validation. |
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.
While I understand the idea of converting everything to a string for the purposes of the check, it feels odd that "myattr" isn't defined as an int. It seems like we should check its "type" as part of the original format (if the format support types), and then check to see if the value is syntactically correct per the pattern/enum/...
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 SHOULD be defined as cetype
of integer
"required": ["myattr"] | ||
} | ||
``` | ||
##### Json Event |
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.
JSON
- Description: Identifies the [Attrschema](#attrschema-document) that attributes of | ||
the current CloudEvent adhere to. | ||
|
||
Incompatible changes to the schema SHOULD be reflected by a different URI. See |
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.
Not sure this first sentence is needed since it's not saying what it's incompatible with. Either this URI describes the current attributes or not.
|
||
#### Examples | ||
Here is an example attrschema document | ||
```json |
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.
Can we add cetype
and format
to the example below?
There's a lot of work here, but this unfortunately overlaps quite a bit with the work we've done on Discovery, especially in the area of message definitions. The snippet below is code generated based out of Microsoft Azure Event Grid metadata for the Azure Storage service on the latest status of the discussion in the discovery group and uses the CloudEvents/1.0 format for a message description. We obviously should avoid having competing models for this. The formats have a slot in the discovery spec, but the prose hasn't been written, yet. I have formal schema definitions for the CloudEvents format, though: https://github.com/clemensv/cloudevents-discovery-interop/blob/master/src/Azure.CloudEvents.Discovery/specs/ce_discovery.yaml#L618 "Microsoft.Storage.BlobCreated": {
"schemaurl": "https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/specification/eventgrid/data-plane/Microsoft.Storage/stable/2018-01-01/Storage.json",
"id": "Microsoft.Storage.BlobCreated",
"epoch": 0,
"self": "https://discovery.example.com/groups/Microsoft.storage/definitions/Microsoft.Storage.BlobCreated",
"description": "Raised when a blob is created.",
"origin": "https://azure.com",
"format": "CloudEvents/1.0",
"metadata": {
"id": {
"required": true
},
"type": {
"value": "Microsoft.Storage.BlobCreated",
"required": true
},
"time": {
"required": true
},
"source": {
"value": "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{storageAccountName}",
"required": false
},
"dataschema": {
"value": "https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/specification/eventgrid/data-plane/Microsoft.Storage/stable/2018-01-01/Storage.json",
"required": true
},
"datacontenttype": {
"value": "application/json",
"required": true
}
}
}, |
@clemensv Thanks for sharing your thoughts on this! |
I think we can close this |
Closing per @sasha-tkachev's comment. Please let me know if we need to reopen it |
Technical definition is complete, only left to add an abstract