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

fix: limit API calls when generating parachain #299

Merged
merged 4 commits into from
Sep 6, 2024
Merged

Conversation

AlexD10S
Copy link
Collaborator

In the pop new parachain command, we perform several sequential operations to gather repository information based on the user's selection:

  1. Fetch the template license.
  2. Retrieve latest releases to display the user the last 3.
  3. Fetch commit hashes: For each of these three releases, we retrieve the commit hash associated with the specified tag in the GitHub repository.
◇  Select a template provider: 
│  Pop 
│
◇  Select the type of parachain:
│  Standard 
│
⚙  Template License: Unlicense
│  
◆  Select a specific release:
│  ● Polkadot v1.14 (polkadot-v.1.14.0 / 4a6e8ef)
│  ○ Polkadot v1.13 
│  ○ Polkadot v1.12 
└  

This process results in a total of five API calls before we even generate the parachain. If the user performs this operation twice in a short period, it results in 10 API calls, which could quickly hit rate limits on the API:

Error: HTTP status client error (403 Forbidden) for url (https://api.github.com/repos/org/project/license)

The limit per hour seems high enough 60 requests per hour, but it seems it also restricts the call per minute.
I reached the limit while working on an integration which means I got stuck and can generate more parachains for a certain period of time. Same happened to another user: #237 (comment)

Solution
By default, the command won't fetch the license (since it's unlikely to change and can be hardcoded) or the commit hashes, cutting the API queries down to just one. If users still want to verify this info, they can use the --verify flag. This approach keeps the "Don't Trust, Verify" principle intact while reducing API calls for most users, making parachain generation smoother and avoiding rate limit issues.
I couldn’t find a way to fetch all the information in a single call, which would be ideal, so I think this is the best approach for now. However, I’m open to other ideas if you have any suggestions.

Closes: #237

@AlexD10S
Copy link
Collaborator Author

@brunopgalvao , could we incorporate this into the documentation?

Copy link

codecov bot commented Aug 29, 2024

Codecov Report

Attention: Patch coverage is 63.15789% with 21 lines in your changes missing coverage. Please review.

Project coverage is 70.26%. Comparing base (e829471) to head (15fb89e).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
crates/pop-cli/src/commands/new/parachain.rs 40.00% 20 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main     #299      +/-   ##
==========================================
- Coverage   70.28%   70.26%   -0.03%     
==========================================
  Files          52       53       +1     
  Lines        8657     9131     +474     
  Branches     8657     9131     +474     
==========================================
+ Hits         6085     6416     +331     
- Misses       1636     1731      +95     
- Partials      936      984      +48     
Files with missing lines Coverage Δ
crates/pop-parachains/src/templates.rs 98.20% <100.00%> (+0.27%) ⬆️
crates/pop-cli/src/commands/new/parachain.rs 46.13% <40.00%> (+0.44%) ⬆️

... and 17 files with indirect coverage changes

@brunopgalvao
Copy link
Contributor

LGTM

When running pop new parachain --verify or pop new parachain my-parachain --verify the output screen should have some indication that the --verify option succeeded.

Added to Pop Docs.

@AlexD10S
Copy link
Collaborator Author

AlexD10S commented Aug 30, 2024

LGTM

When running pop new parachain --verify or pop new parachain my-parachain --verify the output screen should have some indication that the --verify option succeeded.

Added to Pop Docs.

Good idea. What do you think is the best way to go about it? Maybe a note?

@brunopgalvao
Copy link
Contributor

LGTM
When running pop new parachain --verify or pop new parachain my-parachain --verify the output screen should have some indication that the --verify option succeeded.
Added to Pop Docs.
Good idea. What do you think is the best way to go about it? Maybe a note?

Yes, a note in the output with what was verified would be nice.

Example:

┌   Pop CLI : Generating "my-parachain" using Standard from Pop!
│
◆  Generation complete
│  Version: polkadot-v1.9.0 - ✅ Fetched the latest release of the template along with its license based on the commit SHA for the release (4a6e8ef5cade26e0da1fe74ab8bf3509d7f99d59).
│  
▲  NOTE: the resulting parachain is not guaranteed to be audited or reviewed for security vulnerabilities.
│  Please consult the source repository at https://github.com/r0gue-io/base-parachain to assess production suitability and licensing restrictions.
│  
◆  Next Steps:
│  ● cd into "my-parachain" and enjoy hacking! 🚀
│  ● Use `pop build` to build your parachain.
│  ● Use `pop up parachain -f ./network.toml` to launch your parachain on a local network.
│  
└  Need help? Learn more at https://learn.onpop.io

@AlexD10S
Copy link
Collaborator Author

AlexD10S commented Sep 4, 2024

LGTM
When running pop new parachain --verify or pop new parachain my-parachain --verify the output screen should have some indication that the --verify option succeeded.
Added to Pop Docs.
Good idea. What do you think is the best way to go about it? Maybe a note?

Yes, a note in the output with what was verified would be nice.

Example:

┌   Pop CLI : Generating "my-parachain" using Standard from Pop!
│
◆  Generation complete
│  Version: polkadot-v1.9.0 - ✅ Fetched the latest release of the template along with its license based on the commit SHA for the release (4a6e8ef5cade26e0da1fe74ab8bf3509d7f99d59).
│  
▲  NOTE: the resulting parachain is not guaranteed to be audited or reviewed for security vulnerabilities.
│  Please consult the source repository at https://github.com/r0gue-io/base-parachain to assess production suitability and licensing restrictions.
│  
◆  Next Steps:
│  ● cd into "my-parachain" and enjoy hacking! 🚀
│  ● Use `pop build` to build your parachain.
│  ● Use `pop up parachain -f ./network.toml` to launch your parachain on a local network.
│  
└  Need help? Learn more at https://learn.onpop.io

I added the note that way, I like it

@AlexD10S AlexD10S merged commit 32d5870 into main Sep 6, 2024
16 of 18 checks passed
@AlexD10S AlexD10S deleted the alex/fix-api-limit branch September 6, 2024 07:30
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.

fix: set a License default value per Template
2 participants