-
Notifications
You must be signed in to change notification settings - Fork 122
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
feat: local chain registry endpoint #1204
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this looks good. had one question but the answer seems mostly obvious so approving
chainRegistryFile := filepath.Join(wd, "chain_registry.json") | ||
if _, err := os.Stat(chainRegistryFile); err == nil { | ||
crH := handlers.NewChainRegistry(chainRegistryFile) | ||
r.HandleFunc("/chain_registry", crH.GetChainRegistry).Methods(http.MethodGet) | ||
} else { | ||
log.Printf("chain_registry.json not found in %s, not exposing endpoint.", wd) | ||
} | ||
|
||
chainRegistryAssetsFile := filepath.Join(wd, "chain_registry_assets.json") | ||
if _, err := os.Stat(chainRegistryAssetsFile); err == nil { | ||
crH := handlers.NewChainRegistry(chainRegistryAssetsFile) | ||
r.HandleFunc("/chain_registry_assets", crH.GetChainRegistry).Methods(http.MethodGet) | ||
} else { | ||
log.Printf("chain_registry_assets.json not found in %s, not exposing endpoint.", wd) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we call the same handler here so is it just handling the case where the file could be named either chain_registry.json
or chain_registry_assets
? is there any reason for this explicit handling of both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it uses the same as it just returns the JSON object. Since its specific to chain registry and only do that 1 function, it makes sense to just re-use it vs duplicate code.
If we need to add extra logic in the future I would rather keep it scoped to chain registry then a generic JSON viewer
ref: rollchains/spawn#194
Summary
if the chain_registry.json file is found when running
local-ic start <name>
, it will showcase this in an easy to query way. This makes it easy to connect wallets and build existing infra on top. Better UX