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

Emit only one event with multiple attributes in cosmos module handler #1626

Merged
merged 2 commits into from Feb 4, 2020

Conversation

NicolasMahe
Copy link
Member

Fixes #1624

New event output:

[ { "key": "module", "value": "runner" }, { "key": "hash", "value": "9CxqDUrJaXj8Vea6kL5dMzyn7sD6nx6htePkbLitNxfq" } ]

Screen Shot 2020-01-31 at 15 11 08


Should we also create a new type of event. Currently we are using "message" that is define by cosmos.

@NicolasMahe NicolasMahe added this to the next milestone Jan 31, 2020
@NicolasMahe NicolasMahe added this to In progress in Sprint weeks 4 & 5 via automation Jan 31, 2020
@NicolasMahe NicolasMahe self-assigned this Jan 31, 2020
Copy link
Contributor

@krhubert krhubert left a comment

Choose a reason for hiding this comment

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

As I remember engine has a query for recaive this event somewhere. Is it still valid?

@NicolasMahe
Copy link
Member Author

As I remember engine has a query for recaive this event somewhere. Is it still valid?

yes it's still working

@antho1404
Copy link
Member

I think we should have events based on the module name instead of message (that is the name used by cosmos to have an event based on the messages processed) so if we want specific events we should make them dedicated for what we need.

execution: [{ key: "hash", value: "xxxx" }, { key: "action", value: "create" }]

This way we will be able to query all resources that has been created/updated and we can always query multiple resources with AND or OR in the query. https://docs.tendermint.com/master/rpc/#/Websocket/subscribe

To avoid conflicts we could prefix the events with mesg so mesg:execution, mesg:service.

Another solution is to also embed the action in the event name mesg:execution:create. We would still be able to query all execution related events with the CONTAINS instruction but might be harder.

@NicolasMahe NicolasMahe added this to In progress in Sprint weeks 6 & 7 via automation Feb 4, 2020
@krhubert krhubert merged commit d877d99 into dev Feb 4, 2020
Sprint weeks 6 & 7 automation moved this from In progress to Done Feb 4, 2020
@krhubert krhubert deleted the fix/cosmos-events branch February 4, 2020 11:38
@NicolasMahe NicolasMahe mentioned this pull request Feb 4, 2020
3 tasks
@NicolasMahe
Copy link
Member Author

I think we should have events based on the module name instead of message (that is the name used by cosmos to have an event based on the messages processed) so if we want specific events we should make them dedicated for what we need.

execution: [{ key: "hash", value: "xxxx" }, { key: "action", value: "create" }]

This way we will be able to query all resources that has been created/updated and we can always query multiple resources with AND or OR in the query. https://docs.tendermint.com/master/rpc/#/Websocket/subscribe

To avoid conflicts we could prefix the events with mesg so mesg:execution, mesg:service.

Another solution is to also embed the action in the event name mesg:execution:create. We would still be able to query all execution related events with the CONTAINS instruction but might be harder.

let's continue the discussion when the codebase is updated to cosmos v0.38 and we refactor the sdk to cosmos modules. See #1639

@NicolasMahe NicolasMahe added the release:fix Pull requests that fix something label Feb 5, 2020
@antho1404 antho1404 mentioned this pull request Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix Pull requests that fix something
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Group event attributes
3 participants