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

remove typed event usage to keep consistency #13094

Closed
tac0turtle opened this issue Aug 30, 2022 · 7 comments
Closed

remove typed event usage to keep consistency #13094

tac0turtle opened this issue Aug 30, 2022 · 7 comments

Comments

@tac0turtle
Copy link
Member

Summary

Currently there are a few places that use typed events and the majority do not. To keep consistency we should remove typed events until the repo is ready for migration.

Once the team is ready to migrate the repo we can bring them back and roll them out all at once.

@aaronc
Copy link
Member

aaronc commented Aug 30, 2022

Why not starting using typed events more widely?

@tac0turtle
Copy link
Member Author

because there are issues with typed events like you mentioned in the issue like performance.

@aaronc
Copy link
Member

aaronc commented Aug 30, 2022

What other issues? Why don't we just address them holistically as we're improving events?

x/authz, x/group, and x/nft are the only modules I know of using typed events. They're newer modules anyway and trying to reflect this newer design. Unless we want to abandon typed events altogether, I would say we leave the events in these modules alone and just fix any issues.

@tac0turtle
Copy link
Member Author

authz, groups, and nft should not have used typed events as they were not implemented in the repo. If they were the whole repo would be using them.

why cant we be consistent in the sdk?

groups uses orm and a different module structure. we should aim to be consistent in the sdk.

@aaronc
Copy link
Member

aaronc commented Aug 30, 2022

authz, groups, and nft should not have used typed events as they were not implemented in the repo. If they were the whole repo would be using them.

They were implemented in the repo and the plan was to migrate all events to them, there just wasn't time to do that. Are we backtracking on the plan for typed events altogether?

The performance issue relating to JSON marshaling was raised by one user, but I haven't seen actual numbers. There are alternate ways that could be more performant. We should just come up with a design to address this.

groups uses orm and a different module structure. we should aim to be consistent in the sdk.

The plan was always to adopt ORM and a new module structure more widely as well. Groups was an experiment in that direction. Instead of going backwards to the old approach just for consistency's sake, I'd rather see us figure out where we want to move towards and spend time on that.

@aaronc
Copy link
Member

aaronc commented Aug 30, 2022

I propose #11278 as an alternative to this.

@tac0turtle
Copy link
Member Author

I propose #11278 as an alternative to this.

this seems dependent on pulsar. that work is still unknown when it will land.

Ill resurrect the typed events work. I thought since it was never delivered it wasn't ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants