cmd/baseserver, */rest: allow baseserver to choose which handlers to use #207
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make handlers easily configurable to use in cmd/baseserver/main.go. This way client users can trivially change what functionality they'd like. It involves moving ServeCmd out of client/rest to */main.go and lets client/rest become a bazaar for available mux.Router registrars. Updates #200
|
/cc @ethanfrey |
ethanfrey
reviewed
Aug 3, 2017
| // GET: /keys/{name} | ||
| // POST, PUT: /keys/{name} | ||
| // DELETE: /keys/{name} | ||
| func (k *Keys) RegisterAllCRUD(r *mux.Router) error { |
There was a problem hiding this comment.
Nice job pulling out these regsiter functions.
| func serve(cmd *cobra.Command, args []string) error { | ||
| router := mux.NewRouter() | ||
|
|
||
| routeRegistrars := []func(*mux.Router) error{ |
There was a problem hiding this comment.
I like the flexibility of this function very much. But still don't want to have to add all this code in every app. The only thing that changes between apps is routeRegistrars.
For DRY, I would prefer adding some global way to set the routeRegistrars from main.go (calling a function, directly setting a public variable, ...) but have the rest of this file back in client/rest
|
|
||
| // RegisterAll is a convenience function to | ||
| // register all the handlers in this package. | ||
| func RegisterAll(r *mux.Router) error { |
There was a problem hiding this comment.
A bit verbose having the three functions, but very, very awesome for enabling remixing in the different applications.
I like it.
|
Okay, maybe serve in main.go is the best approach, since there is little logic there, and it lets each app create it's own server stack - set up https, middleware, etc.. as needed. I am convinced. |
|
Thanks for the review and merge @ethanfrey! |
mattverse
referenced
this pull request
in mattverse/cosmos-sdk
Apr 20, 2022
fix: Correctly compute post-clawback end time.
alexanderbez
added a commit
that referenced
this pull request
May 26, 2022
* Add proto for Clawback Vesting Account * Change field to int * Change field to int64 * add place holder for clawback * add handler and basic msgs methods * add clawback vesting account logic * improvements * fix: Dev/clawback test debug (#198) * add tests * add clawback before starttime test * clean imports * Printlns for the clawback testing * Fix tests and logic * Fix the gas exhaustion vuln * Remove printlns * Delete extraneous account set * chore: add table driven test cases to clawback (#199) * add table driven tests * Modify test cases * More detail in error message * Update x/auth/vesting/types/vesting_account_test.go * Update x/auth/vesting/types/vesting_account_test.go Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Dev Ojha <dojha@berkeley.edu> Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> * fix: cherry pick agoric's cosmos sdk#207 (#203) * cherry pick compute endtime fix * Merge branch 'osmosis-main' into mattverse/clawback Co-authored-by: mattverse <mattpark1028@gmail.com> Co-authored-by: Matt, Park <45252226+mattverse@users.noreply.github.com> * feat: add clawback cli and tests (#207) * add clawback cli and tests * Update x/auth/vesting/client/cli/tx.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update x/auth/vesting/client/cli/tx.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Update x/auth/vesting/client/cli/tx.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * minor fix * Minor fix * Update x/auth/vesting/client/cli/cli_test.go Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> * Change from code review from Alex and Dev Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
yihuang
added a commit
to yihuang/cosmos-sdk
that referenced
this pull request
Mar 25, 2024
Solution: - remove the api changelog
dudong2
referenced
this pull request
in b-harvest/cosmos-sdk
Oct 17, 2024
Solution: - remove the api changelog
dudong2
referenced
this pull request
in b-harvest/cosmos-sdk
Oct 30, 2024
Solution: - remove the api changelog
mmsqe
pushed a commit
to mmsqe/cosmos-sdk
that referenced
this pull request
Dec 12, 2024
Solution: - remove the api changelog
mmsqe
pushed a commit
to mmsqe/cosmos-sdk
that referenced
this pull request
Dec 12, 2024
Solution: - remove the api changelog
mmsqe
added a commit
to mmsqe/cosmos-sdk
that referenced
this pull request
Dec 12, 2024
This reverts commit 56cb96c.
mmsqe
added a commit
to mmsqe/cosmos-sdk
that referenced
this pull request
Jan 17, 2025
* Revert "Problem: CacheWrapWithTrace api is not used (cosmos#207)" This reverts commit 56cb96c. * resolve * cleanup * fix cyclic dependency * fix test * better equavalence --------- Co-authored-by: HuangYi <huang@crypto.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make handlers easily configurable to use in cmd/baseserver/main.go.
This way client users can trivially change what functionality they'd
like.
It involves moving ServeCmd out of client/rest to */main.go
and lets client/rest become a bazaar for available mux.Router
registrars.
Updates #200