feat: added "responseTo" field in worker emitted events #141
Conversation
DuckBoss
force-pushed
the
jajerome/reponse-to-in-events
branch
from
61fc821 to
37cc3c2
Compare
DuckBoss
force-pushed
the
jajerome/reponse-to-in-events
branch
2 times, most recently
from
7c8c84c to
93be077
Compare
| @@ -62,6 +63,7 @@ | |||
| <arg type="s" name="worker" /> | |||
| <arg type="u" name="name" /> | |||
| <arg type="s" name="message_id" /> | |||
| <arg type="s" name="response_to" /> | |||
There was a problem hiding this comment.
We can avoid breaking this API by putting the response_to field below message.
DuckBoss
force-pushed
the
jajerome/reponse-to-in-events
branch
4 times, most recently
from
fe7dc7e to
8f56c5d
Compare
DuckBoss
changed the title
DuckBoss
force-pushed
the
jajerome/reponse-to-in-events
branch
5 times, most recently
from
fa45061 to
d3ac092
Compare
Signed-off-by: Jason Jerome <[email protected]>
DuckBoss
force-pushed
the
jajerome/reponse-to-in-events
branch
from
d3ac092 to
8bb097c
Compare
worker/echo/main.go
Outdated
| if err := w.EmitEvent( | ||
| ipc.WorkerEventNameWorking, | ||
| rcvId, | ||
| echoId, |
There was a problem hiding this comment.
This is interesting. Prior to this change, each event emitted by the worker
would have the message ID of the received message in the first field, resulting
in output that looks like:
2023/09/15 12:52:45 echo: 53a8d7d5-e5cb-495c-9d18-2e055d566638: BEGIN: {}
2023/09/15 12:52:45 echo: 53a8d7d5-e5cb-495c-9d18-2e055d566638: WORKING: {"message":"echoing [104 101 108 108 111]"}
2023/09/15 12:52:45 echo: 53a8d7d5-e5cb-495c-9d18-2e055d566638: END: {}
Now however, the BEGIN and END events have the originating message ID first:
2023/09/15 12:52:45 echo: 53a8d7d5-e5cb-495c-9d18-2e055d566638: BEGIN: : {}
2023/09/15 12:52:45 echo: 80a83707-f982-4e82-82e5-90f06198f70e: WORKING: 53a8d7d5-e5cb-495c-9d18-2e055d566638: {"message":"echoing [104 101 108 108 111]"}
2023/09/15 12:52:45 echo: 53a8d7d5-e5cb-495c-9d18-2e055d566638: END: : {}
The two events handled by the worker package don't have any awareness to the
"response_to" field, so they don't print anything for that value. The WORKING
event is emitted by the echo worker, and it has created a new UUID for a
response message and puts that into the value of WORKING.
Overall, I don't think this is a significant flaw in this PR. But the way its
handled in this echo worker feels conflicted. I think it should continue to emit
the WORKING event, using the message ID of the received message as the first
field to the EmitEvent function. The "responseTo" function parameter has not
changed its meaning; it is still the value of the "response_to" field in the
received message.
There was a problem hiding this comment.
Thanks for the feedback!
I've reverted the commit that made this change so now it should continue to emit the message ID of the received message in the EmitEvent function for the WORKING event.
DuckBoss
force-pushed
the
jajerome/reponse-to-in-events
branch
from
8bb097c to
cb0e2b5
Compare
Currently uses cherry-picked commits from dependent PR: #140 which needs to be removed when the dependent PR gets approved and merged.