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/init blocking #1660

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix/init blocking #1660

wants to merge 2 commits into from

Conversation

elizabethengelman
Copy link
Contributor

@elizabethengelman elizabethengelman commented Oct 8, 2024

What

  • moves the init integration test into the integration dir
  • makes the init command async, and then uses the async reqwest client.

Why

I was seeing this error when running contract init on main:

thread 'main' panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.39.2/src/runtime/blocking/shutdown.rs:51:21:
Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This happens because we were trying to use the blocking reqwest client inside of an async runtime, since all of the cli is in an tokio runtime. This doesn't work well together, so I've changed the init command to be async and then we're able to await the HTTP calls.

Known limitations

This won't be an issue once https://github.com/stellar/stellar-cli/pull/1628/files is merged - but we'll still want to move the init test into the integration dir - I think that this test was previously not being run.

@elizabethengelman elizabethengelman marked this pull request as ready for review October 8, 2024 18:55
Copy link
Member

@willemneal willemneal left a comment

Choose a reason for hiding this comment

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

Good catch

@leighmcculloch
Copy link
Member

Because #1628 is merged, does anything from this change still need to be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog (Not Ready)
Development

Successfully merging this pull request may close these issues.

3 participants