Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Hierarchical routing for SDK Apps #770

Closed
mossid opened this issue Apr 1, 2018 · 5 comments
Closed

Hierarchical routing for SDK Apps #770

mossid opened this issue Apr 1, 2018 · 5 comments
Labels
S:proposed

Comments

@mossid
Copy link
Contributor

mossid commented Apr 1, 2018

Currently, Router in Baseapp is implemented as a simple list of routes, so the msgs are routed to the handlers those are assigned to exactly the same path with the msg's type.

app.Router()
  .AddRoute("bank", bank.NewHandler())
  .AddRoute("stake", stake.NewHandler())

Originally, Router was designed to be hierarchical, so something like this could be done:

app.Router()
  .AddRoute("bank", bank.NewHandler())
  .AddRoute("bank/issue", mybank.NewHandler()) // overwriting handler for specific msg
@cwgoes
Copy link
Contributor

cwgoes commented Apr 2, 2018

Interesting. Do you have specific examples in mind where an SDK user would want to overwrite particular module handlers like this?

@rigelrozanski
Copy link
Contributor

Yeah cool thanks @mossid for clearing this one up, the design was confusing as to where one might use bank/issue - I think once this is implemented we could add an example of an overwrite to democoin

@cwgoes
Copy link
Contributor

cwgoes commented Apr 12, 2018

Awaiting discussion at SDK design meeting.

@cwgoes
Copy link
Contributor

cwgoes commented Apr 16, 2018

Tabled by SDK design meeting to post-launch.

@ValarDragon ValarDragon mentioned this issue Jul 17, 2018
Closed
ValarDragon added a commit that referenced this issue Sep 15, 2018
@ValarDragon
Add a name field to the message type
7fa3579 
This is to facillitate ease of implementing #1406. (Tags for messages
could then be added dynamically)

Ultimately once we make the router support hiearchical routing, (#770)
we can then remove the name field and just the parse info for tags from that.

Until then, we can parse the tag name as
`fmt.Sprintf("%s %s", msg.Type(), msg.Name())`
@ValarDragon ValarDragon mentioned this issue Sep 15, 2018
Merged
4 tasks
cwgoes pushed a commit that referenced this issue Sep 17, 2018
@ValarDragon @cwgoes
Merge PR #2343: Add a name field to the message type
65137f6 
This is to facillitate ease of implementing #1406. (Tags for messages
could then be added dynamically)

Ultimately once we make the router support hiearchical routing, (#770)
we can then remove the name field and just the parse info for tags from that.

Until then, we can parse the tag name as
`fmt.Sprintf("%s %s", msg.Type(), msg.Name())`
@jackzampolin jackzampolin changed the title Hierarchical routing in Baseapp Hierarchical routing for SDK Apps Oct 12, 2018
@jackzampolin
Copy link
Member

Going to close this issue. Please reopen with more information if applicable.

# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
S:proposed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@martindale @ValarDragon @jackzampolin @mossid @rigelrozanski @cwgoes