feat: base CloudEvent class as per v1 specs, including attribute validation

[PlugaruT]

Changes

The first PR towards a full rewrite of the library. What's included in this PR:

• Support for v 1.0.2 of the specs with better validation errors and full support for custom extension attributes, including their validation.
• Full typing support
• Better documentation for each class and methods

One line description for the changelog

• Tests pass
• Appropriate changes to README are included in PR

[xSAVIKx]

Hey @PlugaruT, thx for PR. Please take a look at comments and suggestions.

[PlugaruT]

There's a bit of a problem with mypy it seems, it doesn't like the dict[str, Any] for the attributes and also explicit str return for id field for example, since it's a required field and it's str.
I tried to play with TypedDict, but that one doesn't allow for arbitrary keys, but we need to support custom extensions. So the only way to "fix" this is to suppress the error? Let me know what you think.

[xSAVIKx]

There's a bit of a problem with mypy it seems, it doesn't like the dict[str, Any] for the attributes and also explicit str return for id field for example, since it's a required field and it's str. I tried to play with TypedDict, but that one doesn't allow for arbitrary keys, but we need to support custom extensions. So the only way to "fix" this is to suppress the error? Let me know what you think.

There's a total option for TypedDict. You can read more in MyPy docs: https://mypy.readthedocs.io/en/stable/typed_dict.html#mixing-required-and-non-required-items and in the PEP: https://peps.python.org/pep-0589/#totality

We can have RequiredCloudEventAttributes(TypedDict) and then have `CloudEventAttributes(RequiredCloudEventAttributes, total=False). This will also eliminate part of the validations. E.g. typings and presence of required attributes can be checked by the typing system and we can have only optional runtime validation. This may be a good approach especially where speed is more important than runtime validation.

[xSAVIKx]

@PlugaruT, please don't use force push. The only merge strategy allowed in this repository is Squash and merge, so it's OK to have as many commits as you want and the way you want them.

force push makes it pretty hard to follow the review process in GitHub unfortunately

[PlugaruT]

There's a bit of a problem with mypy it seems, it doesn't like the dict[str, Any] for the attributes and also explicit str return for id field for example, since it's a required field and it's str. I tried to play with TypedDict, but that one doesn't allow for arbitrary keys, but we need to support custom extensions. So the only way to "fix" this is to suppress the error? Let me know what you think.

There's a total option for TypedDict. You can read more in MyPy docs: https://mypy.readthedocs.io/en/stable/typed_dict.html#mixing-required-and-non-required-items and in the PEP: https://peps.python.org/pep-0589/#totality

We can have RequiredCloudEventAttributes(TypedDict) and then have `CloudEventAttributes(RequiredCloudEventAttributes, total=False). This will also eliminate part of the validations. E.g. typings and presence of required attributes can be checked by the typing system and we can have only optional runtime validation. This may be a good approach especially where speed is more important than runtime validation.

Yes, but the problem is with arbitrary keys, like extension names, since the TypedDict doesn't accept random keys besides the ones defined. If I try to define a CloudEvent instance with an extension, mypy complains:

src/cloudevents/core/v1/event.py:252: error: Extra key "customextension" for
TypedDict "CloudEventsAttributes"  [typeddict-unknown-key]
        e = CloudEvent({
                       ^
Found 2 errors in 1 file (checked 1 source file)

There's a whole discussion going on here https ://github.com/python/mypy/issues/ 4617 (intended not to link the PR's). So yes, it will "work" for the known attributes, but users will have to set the flag when they'll use this. Up to you what we want to do here.

[PlugaruT]

@xSAVIKx, I addressed all the comments, except the mypy one that is a bit tricky. Let me know your thoughts

[xSAVIKx]

@PlugaruT overall LGTM. I'd suggest we modify the error gathering a bit, but otherwise that's good.

[xSAVIKx]

@PlugaruT PTAL at the comments and suggestions. Doing good 👍

[xSAVIKx]

I don't recall this being required. Where did you get this requirement from?

[PlugaruT]

time attribute should respect RFC3339 which requires having timestamps with timezone attached.

[PlugaruT]

I think here, we have 2 options, either default to UTC, but that would be a controversial behaviour, as we can't default the timezone of the clients, or, make it an explicit requirement.
From my experience, using Python SDK and Java SDK, Java SDK will break when trying to have a naive datetime string. See here for example, time is defined as OffsetDateTime which means exactly this, timestamp with a timezone offset

[PlugaruT]

@xSAVIKx can you also merge please, I don't have the perms to do it. Thanks