biomunky / newsapi

An API for interacting with the News API
MIT License
7 stars 2 forks source link

Changes for async API (non-blocking reqwest) #27

Closed leodutra closed 3 years ago

leodutra commented 4 years ago

Implements async send() + async fetch_resource() and implements changes for the new usage (+examples).

This change is required to enable the API to be used in async scopes using async runtime, like actix-web 2.0. Otherwise, a "trying to block into async thread" [sic] error occurs.

The examples use futures crate to keep the code as close to the last version as possible.

I was able to test into my own private chatbot, it's working as expected.

This should probably become a 4.x version.

biomunky commented 4 years ago

Hi, can you bump the version. Let’s get this in.

biomunky commented 4 years ago

When I run the examples I'm getting a panic

Running target/debug/examples/get_sources thread 'main' panicked at 'not currently running on the Tokio runtime.', /Users/dan/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.2.16/src/runtime/handle.rs:69:9 note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

biomunky commented 4 years ago

The panic can be solved by doing the following.

Add tokio to Cargo.toml

tokio = { version = "0.2", features = ["full"] }

Modify the examples to use the tokio main macro. Using full as this is recommended in the docs when starting out.

#[tokio::main]
async fn main() { ... }

While the async stuff is cool, I'm not sure I'm comfortable with throwing the synchronous client away. Perhaps a refactor is in order so that we can support async and sync clients?

leodutra commented 4 years ago

Sounds good. It's late my time, and sorry for the delay, but I'll do it ASAP.

biomunky commented 4 years ago

Sounds good. It's late my time, and sorry for the delay, but I'll do it ASAP.

No rush, this is supposed to be fun rather than work.

leodutra commented 3 years ago

Hey @biomunky. Sorry, I totally missed this PR. I kept the sync API, added some _sync suffix for the 2 sync/ async methods we have, fixed the examples and repeated them for sync versions.

biomunky commented 3 years ago

cool, likewise missed :) will look tomorrow.

biomunky commented 3 years ago

I'm going to merge and then make some more changes before releasing a new version.