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

Alternative to #741 - dealing with errors #763

Merged
merged 1 commit into from
Feb 5, 2021
Merged

Conversation

duglin
Copy link
Collaborator

@duglin duglin commented Jan 23, 2021

fixes: #741

Signed-off-by: Doug Davis dug@us.ibm.com

@duglin
Copy link
Collaborator Author

duglin commented Jan 23, 2021

/cc @yordis

@yordis
Copy link

yordis commented Jan 23, 2021

@duglin thank you so much for such added up.

I would suggest adding the intention of the Adapters: https://github.com/cloudevents/spec/tree/v1.0.1/adapters since those Adapters do decide the shape and meaning of a particular CloudEvent.

Hopefully, that reset the conversation that I started with the Error Proposal Adapter (and people understand that it is a suggestion) or make it clear to outsider folks like me to avoid such conversations if CloudEvents is not interested in such conversations.

I still believe that suggesting Adapters as you do today for other things will be a huge benefit in the long term for the ecosystem, so making clear what your intention is will be a huge win.

Thanks a lot.

@yordis
Copy link

yordis commented Jan 23, 2021

Same for the Extensions section: https://github.com/cloudevents/spec/tree/v1.0.1/extensions

fixes: cloudevents#741

And mention the adpaters

Signed-off-by: Doug Davis <dug@us.ibm.com>
@duglin
Copy link
Collaborator Author

duglin commented Jan 26, 2021

@yordis I added a paragraph about the adapters, but I'm not sure about the extensions since those are similar to the normal CE attributes... we just define their meaning and syntax, but we don't get into the specific values that should be used. See what you think.

@yordis
Copy link

yordis commented Jan 26, 2021

but we don't get into the specific values that should be used.

Personally, putting aside if I defined the spec or not, this is where I wish we don't do this for the exact same reasons CloudEvents exists for general events.

From my experience, the spec that I came up with, is the "minimal" spec that we could use and allow people to build upon, and at least we have build tools around such conventions or swap companies, ecosystems, and so on without having to re-learn things.

I don't know, take my proposal or somebody else take the lead on trying to align the ecosystem, but I would love to have such a thing from CloudEvents, take the current "suggestions of adapters", at least people will take a look and have "some alignment" vs no alignment at all 😃

@duglin
Copy link
Collaborator Author

duglin commented Jan 27, 2021

I'm not sure I'm following. The adapters talk about the values that should be used in the CE attributes because they're specifically designed to show how you might convert something like a github event into a CloudEvent. And in that case we have to explain where to get the CE attribute values from - otherwise you can't create a CloudEvent. So, we're talking about a very specific situation with very specific event data - it's not generic.

In the case of creating an "Error CloudEvent", can you elaborate on how the proposal would work? When I look at your PR, in particular the example at the end, a receiver would only know that the CE was an error if it knew that com.straw-hat-team.payment.insufficient_fund was an error type. And if it knew that, then it seems like it would know how to parse the data attribute to get the error specific data - so I'm not sure what value the extra attribute provide.

I think it might help me if you could explain what the high-level interop goal is that you're trying to achieve. Is it that you want a standard event format for errors? Meaning, a standard set of properties for data for all errors? Or perhaps explain how the receiver of the CE in your sample is helped by the presence of those extra attributes under data and is the benefit to the middleware trying to get the event to the proper destination or is it for the final destination?

@duglin
Copy link
Collaborator Author

duglin commented Feb 5, 2021

Approved on the 2/4 call

@duglin duglin merged commit 30b1b9a into cloudevents:master Feb 5, 2021
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.

2 participants