-
Notifications
You must be signed in to change notification settings - Fork 419
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
Event payload attributes on batch requests #945
Comments
Would this mean that some events will have |
We could remove |
From what I've worked with this plugin, I couldn't understand why we need the |
For instance for attachment you need it to know on which record you want to assign the attachment to. We can probably have helper that let us get this information from the URI but I like the idea of having them already there in the payload. If event's are different ones I don't mind having a different payload because a plugin most of the time subscribe to one event at the time, so we won't need different event handling. Also if |
Well, but are the events different ones? It seems like this proposal is to change |
I think there is some confusion here. The problem concerns the events that have several impacted records. |
Well, I'm still confused. Let's consider the following cases:
Let's say this generates an event like:
Now consider:
This request is identical to the first one, so it should generate the same events. Agreed? You're saying that the second one, if and only if it contains two PUT requests, should be missing My argument is that whoever authored the plugin doesn't care whether the events came from someone making a batch request with two PUTs, or someone making two separate PUT requests. The events should be the same, and have the same semantics, in both cases. So if you want to remove the |
In investigating this, I noticed that the |
OK, never mind that last comment -- I think I confused myself. For a given |
On batch requests, an event is sent for each (parent id / resource / action), and the list of objects is provided in
event.impacted_records
. But the content ofevent.payload
makes very little sense in this context, and can lead to serious confusions:I propose that when there are more than one impacted object then we drop the
${resource_name}_id
and theuri
keys.Thoughts?
The text was updated successfully, but these errors were encountered: