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

Pull chain data from Chain Registry, Add paths new cmd, & Improve CLI experience/quick-start guide #557

Merged
merged 13 commits into from
Feb 4, 2022

Conversation

jtieri
Copy link
Member

@jtieri jtieri commented Jan 18, 2022

lens now contains the facilities for fetching chain metadata from the chain-registry.

This PR removes the fetch cmd tree from the relayer in favor of using lens for fetching this chain metadata. Users can now specify which chains they want to pull data for in rly chains add [chain names]
e.g. rly chains add osmosis cosmoshub

Because the chain-registry does not contain the necessary metadata for paths we still need to maintain this data ourselves but instead of using the fetch cmd tree users can use rly paths fetch which will pull the paths for every chain they have configured via rly chains add

#564 adds a new command rly paths new [src-chain-id] [dst-chain-id] [path-name] [flags] which will generate a new blank path between src and dst in your config.yaml file

Closes #538

@jtieri jtieri requested a review from jackzampolin as a code owner January 18, 2022 17:39
@jtieri jtieri changed the title Pull Chain data from Chain Registry & improve cli experience Pull chain data from Chain Registry & improve cli experience/quick-start guide Jan 18, 2022
@boojamya
Copy link
Contributor

Want to note that I experienced this issue (#562) when adding chains today.

@jtieri
Copy link
Member Author

jtieri commented Jan 20, 2022

Want to note that I experienced this issue (#562) when adding chains today.

Interesting, maybe we can increase the rate limit enough to not hit these errors using authenticated requests.

Error: GET https://api.github.com/repos/cosmos/chain-registry/git/trees/master: 403 API rate limit exceeded for 172.xxx.xxx.xx. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.

https://github.com/google/go-github#authentication

Maybe this is a per user basis though? Managing tokens as a user seems like bad UX so perhaps we do want to clone the repo and traverse the files locally to move from making multiple calls to the GitHub API to only one every time rly chains add is called

@boojamya
Copy link
Contributor

boojamya commented Jan 20, 2022

Read through some of the docs. It's confusing b/c its different on how you set it up and github user status. I can't imagine I hit the rate limit as and individual user. Without an enterprise account I get 5000 requests per hour. ¯_(ツ)_/¯

So... check this out:

When using the built-in GITHUB_TOKEN in GitHub Actions, the rate limit is 1,000 requests per hour per repository. For organizations that belong to a GitHub Enterprise Cloud account, this limit is 15,000 requests per hour per repository.

For unauthenticated requests, the rate limit allows for up to 60 requests per hour. Unauthenticated requests are associated with the originating IP address, and not the user making requests.

(From: https://docs.github.com/en/rest/overview/resources-in-the-rest-api#rate-limiting)

So I guess it depends on how we implemented the API call... and it could be repo based.

I do think pulling the whole chain registry repo down is a good quick fix though. OR pointing to URL specifically.

@jtieri jtieri mentioned this pull request Jan 21, 2022
jackzampolin and others added 2 commits January 24, 2022 08:43
* add command to create a new blank path in the config

* add retries & cleanup CreateClients

* use tagged lens version
@jtieri jtieri changed the title Pull chain data from Chain Registry & improve cli experience/quick-start guide Pull chain data from Chain Registry, Refactor path generation, & Improve CLI experience/quick-start guide Jan 25, 2022
@jtieri jtieri changed the title Pull chain data from Chain Registry, Refactor path generation, & Improve CLI experience/quick-start guide Pull chain data from Chain Registry, Add paths new cmd, & Improve CLI experience/quick-start guide Jan 28, 2022
@jtieri jtieri requested a review from boojamya February 4, 2022 21:23
Copy link
Contributor

@boojamya boojamya left a comment

Choose a reason for hiding this comment

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

love these features.

@boojamya boojamya merged commit b11c34f into main Feb 4, 2022
@boojamya boojamya deleted the justin/use-chain-registry branch February 4, 2022 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor fetching chain configuration to use chain-registry
3 participants