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

fix: SetExtension cannot override core attributes #720

Merged
merged 1 commit into from
Oct 28, 2021

Conversation

embano1
Copy link
Member

@embano1 embano1 commented Oct 4, 2021

Fail if SetExtension is called with a core attribute.

TODO:

  • Implement for V03
  • Update func docs

Closes: #709
Signed-off-by: Michael Gasch mgasch@vmware.com

@embano1
Copy link
Member Author

embano1 commented Oct 4, 2021

Up for discussion:

Where can I reference the V03 and V1 spec attributes instead of creating a custom map? I tried using Attributes() from spec but that introduces a circular dep.

}

// TODO: not safe, can be bypassed with a ce-ce- prefix
trimmed := strings.TrimPrefix(key, "ce-")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed, if the key has a - it is not a valid extension per the test on 70.

@n3wscott n3wscott closed this Oct 4, 2021
@n3wscott n3wscott reopened this Oct 4, 2021
@n3wscott
Copy link
Member

are you going to do the todos here or a follow up?

@embano1
Copy link
Member Author

embano1 commented Oct 22, 2021

Sorry, sorry. Any ideas on the second comment instead of reinventing the event attributes wheel?

v2/event/extensions.go Outdated Show resolved Hide resolved
@embano1 embano1 force-pushed the issue-709 branch 2 times, most recently from 785d926 to 52095dc Compare October 27, 2021 13:12
Copy link
Member

@n3wscott n3wscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, one nit:

v2/event/eventcontext_v03.go Outdated Show resolved Hide resolved
v2/event/eventcontext_v1.go Outdated Show resolved Hide resolved
Closes: cloudevents#709
Signed-off-by: Michael Gasch <mgasch@vmware.com>
@n3wscott n3wscott merged commit 1432760 into cloudevents:main Oct 28, 2021
@embano1 embano1 deleted the issue-709 branch October 29, 2021 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SetExtension seems to override core attributes for http at least.
2 participants