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: update API paths to use API domain #107

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

CharlieC3
Copy link
Member

This updates any endpoints which use a Stacks Blockchain API path to call the correctly configured endpoint for the service. Previously, this bug was hidden because the Stacks Blockchain API and Stacks Core RPC service had to be configured on the same domain. Now that they can be configured on separate domains since release 1.3.0, this bug popped up.

I've deployed and tested this patch and it appears to be running the same as it were at version 1.2.1.

@jcnelson jcnelson self-requested a review January 5, 2024 20:15
Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

Fix the failing tests first

Set blockstackAPIUrl to coreApiUrl to ensure tests are run against the correct endpoint
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f9cbbce) 57.55% compared to head (32e9c3e) 57.42%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
- Coverage   57.55%   57.42%   -0.14%     
==========================================
  Files           8        8              
  Lines         761      761              
==========================================
- Hits          438      437       -1     
- Misses        323      324       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CharlieC3 CharlieC3 requested a review from jcnelson January 5, 2024 21:45
@CharlieC3
Copy link
Member Author

@jcnelson Fixed!

@@ -9,6 +9,7 @@ import { StacksMainnet } from '@stacks/network'
nock.disableNetConnect()

bskConfig.network = new StacksMainnet()
bskConfig.network.blockstackAPIUrl = bskConfig.network.coreApiUrl
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't this be easy to change the env var to STACKS_API_URL etc, vs continuing to use the BLOCKSTACK_API_URL?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe, but it's not a high enough priority for us to invest more resources in at the moment.

@CharlieC3 CharlieC3 requested a review from wileyj January 16, 2024 18:35
Copy link
Contributor

@wileyj wileyj left a comment

Choose a reason for hiding this comment

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

shipit

@wileyj wileyj merged commit 0b2f8fa into stacks-network:master Jan 16, 2024
5 checks passed
@blockstack-devops
Copy link

🎉 This PR is included in version 1.3.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants