Improve helix::client_ext helpers & types in collections #359

Emilgardis · 2023-04-30T00:25:04Z

No description provided.

Emilgardis · 2023-04-30T00:29:19Z

src/helix/client/client_ext.rs

+    ) -> impl futures::Stream<Item = Result<helix::streams::Stream, ClientError<C>>>
+           + Send
+           + Unpin
+           + 'client
+    where
+        T: TwitchToken + Send + Sync + ?Sized,
+    {
+        use futures::StreamExt;
+        futures::stream::iter(ids.chunks(100).map(move |c| {
+            let req = helix::streams::GetStreamsRequest::user_ids(c).first(100);
+            make_stream(req, token, self, std::collections::VecDeque::from)
+        }))
+        .flatten_unordered(None)
+    }


I really like this construct, maybe we can use it for the other calls as well, to make it possible to query as many things as possible.

Should probably move away from returning a impl Stream then, and instead return a TwitchResults<T> or similar which implements it, which we can also use to capture the token better (I think?)

or, we should just hint to the user; "this is a way to query more, do it this way"

Emilgardis · 2023-04-30T00:31:08Z

src/helix/client/client_ext.rs

+    pub fn get_streams_from_ids<T>(
+        &'client self,
+        ids: &'client [&'client types::UserIdRef],
+        token: &'client T,
+    ) -> impl futures::Stream<Item = Result<helix::streams::Stream, ClientError<C>>>


I hate it

I want the lifetime to be more explicit, should figure out what the correct bound is (if this is not maximally permissable, which I suspect it isn't)

Nerixyz · 2024-04-08T15:10:35Z

It's good to include more helpers, esp. for easy pagination.

Seems like we only need to adjust the example:

diff --git a/examples/followed_streams.rs b/examples/followed_streams.rs
index d702260109..b423dfa66c 100644
--- a/examples/followed_streams.rs
+++ b/examples/followed_streams.rs
@@ -1,6 +1,9 @@
+use std::collections::HashMap;
+
 use futures::TryStreamExt;
-use twitch_api::helix::HelixClient;
+use twitch_api::helix::{games::get_games::Game, HelixClient};
 use twitch_oauth2::{AccessToken, UserToken};
+use twitch_types::CategoryId;
 
 fn main() {
     use std::error::Error;
@@ -36,7 +39,7 @@ async fn run() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>>
         .get_followed_streams(&token)
         .try_collect::<Vec<_>>()
         .await?;
-    let games = client
+    let games: HashMap<CategoryId, Game> = client
         .get_games_by_id(
             &streams
                 .iter()
@@ -44,6 +47,8 @@ async fn run() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>>
                 .collect::<Vec<_>>(),
             &token,
         )
+        .map_ok(|game| (game.id.clone(), game))
+        .try_collect()
         .await?;
 
     println!(

Emilgardis · 2024-04-09T13:11:28Z

I have some unpushed changes to this branch incorporating twitch-rs/twitch_types#55, I'll clean it up and push for a review

Emilgardis · 2024-04-09T17:56:07Z

i've pushed the changes

src/helix/endpoints/streams/replace_stream_tags.rs

Emilgardis · 2024-04-09T18:07:16Z

src/helix/client/client_ext.rs

+    /// let users: Vec<helix::users::User> = client
+    ///    .get_users_from_ids(&["1234", "4321"][..].into(), &token).try_collect().await?;
+    /// # Ok(()) }
+    /// ```
+    pub fn get_users_from_ids<T>(
        &'client self,
-        ids: impl AsRef<[&types::UserIdRef]> + Send,
-        token: &T,
-    ) -> Result<Option<helix::users::User>, ClientError<C>>
+        ids: &'client types::Collection<'client, types::UserId>,
+        token: &'client T,
+    ) -> impl futures::Stream<Item = Result<helix::users::User, ClientError<C>>> + Send + Unpin + 'client
    where
        T: TwitchToken + Send + Sync + ?Sized,
    {
-        let ids = ids.as_ref();
-        if ids.len() > 100 {
-            return Err(ClientRequestError::Custom("too many IDs, max 100".into()));
-        }
-        self.req_get(helix::users::GetUsersRequest::ids(ids), token)
-            .await
-            .map(|response| response.first())
+        futures::stream::iter(ids.chunks(100).collect::<Vec<_>>())
+            .map(move |c| {
+                let req = helix::users::GetUsersRequest::ids(c);
+                futures::stream::once(self.req_get(req, token)).boxed()
+            })
+            .flatten_unordered(None)
+            .map_ok(|resp| futures::stream::iter(resp.data.into_iter().map(Ok)))
+            .try_flatten_unordered(None)
    }


I don't like how the examples have to use [..]

I also tried to make the helper functions take impl Into<Collection<'_, T>> but it didn't work. Still going to try to make it work but I suspect we need some the changes made in 2024 edition for lifetime captures behind impls, or we go the TwitchResult route. I suspect the TwitchResult route is what is needed for doing minimal allocations also

It's really only an issue with arrays, no? In real-world code, this would probably be a Vec or a slice already. But I agree that it makes the examples a bit complicated.

examples/channel_information_custom.rs

src/helix/endpoints/chat/get_emote_sets.rs

src/helix/client/client_ext.rs

src/helix/endpoints/search/mod.rs

Nerixyz

Just some copy-paste artifacts.

src/helix/client/client_ext.rs

Emilgardis · 2024-04-15T15:20:23Z

final check wanted, think we can merge this

Nerixyz

Just documentation stuff. For the wrapping, I put four spaces indentation, but three spaces might look better in the docs.

src/helix/endpoints/chat/get_emote_sets.rs

src/helix/endpoints/streams/get_streams.rs

src/helix/client/client_ext.rs

Co-authored-by: nerix <[email protected]>

Emilgardis force-pushed the helpers branch from f4bd5f0 to 1ba6806 Compare April 30, 2023 00:28

Emilgardis commented Apr 30, 2023

View reviewed changes

Emilgardis mentioned this pull request May 7, 2023

Make it possible to construct a Response #369

Merged

Emilgardis changed the title ~~add helper for getting streams~~ Improve helix::client_ext helpers Aug 4, 2023

Emilgardis force-pushed the helpers branch from 1ba6806 to 8adf8a6 Compare January 12, 2024 19:39

Emilgardis mentioned this pull request Apr 7, 2024

chore: remove tmi endpoints #407

Merged

This comment was marked as resolved.

Sign in to view

Emilgardis force-pushed the helpers branch 2 times, most recently from 7ecd9cd to af46a86 Compare April 9, 2024 17:44

Emilgardis commented Apr 9, 2024

View reviewed changes

Nerixyz reviewed Apr 9, 2024

View reviewed changes

Emilgardis force-pushed the helpers branch from c1a3e5c to ed78a47 Compare April 12, 2024 08:05

Nerixyz approved these changes Apr 12, 2024

View reviewed changes

src/helix/client/client_ext.rs Outdated Show resolved Hide resolved

src/helix/client/client_ext.rs Outdated Show resolved Hide resolved

src/helix/client/client_ext.rs Outdated Show resolved Hide resolved

Nerixyz mentioned this pull request Apr 13, 2024

docs: add overview for implemented requests #412

Merged

Emilgardis added 2 commits April 15, 2024 17:05

add helper for getting streams

eac5edd

some touches and make more helpers take an arbitrary amount of items

070655c

Emilgardis force-pushed the helpers branch from cb614a1 to 8f99595 Compare April 15, 2024 15:08

Emilgardis changed the title ~~Improve helix::client_ext helpers~~ Improve helix::client_ext helpers & improve types in collections Apr 15, 2024

Emilgardis changed the title ~~Improve helix::client_ext helpers & improve types in collections~~ Improve helix::client_ext helpers & types in collections Apr 15, 2024

Emilgardis requested a review from Nerixyz April 15, 2024 15:40

Nerixyz reviewed Apr 15, 2024

View reviewed changes

Apply suggestions from code review

788c4ea

Co-authored-by: nerix <[email protected]>

Nerixyz approved these changes Apr 15, 2024

View reviewed changes

Emilgardis force-pushed the helpers branch 2 times, most recently from 62216d0 to 788c4ea Compare April 15, 2024 17:13

Emilgardis enabled auto-merge April 15, 2024 17:13

Emilgardis added this pull request to the merge queue Apr 15, 2024

Merged via the queue into twitch-rs:main with commit 5d94ce8 Apr 15, 2024
16 checks passed

Emilgardis deleted the helpers branch April 15, 2024 17:28

Emilgardis mentioned this pull request Oct 11, 2024

Clarify how or make it easier to pass in collections that are T: Iterator<Item=D> in builder functions for helix requests #374

Closed

Improve helix::client_ext helpers & types in collections #359

Improve helix::client_ext helpers & types in collections #359

Uh oh!

Conversation

Emilgardis commented Apr 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Emilgardis Apr 30, 2023

Choose a reason for hiding this comment

Uh oh!

Emilgardis Apr 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Emilgardis Apr 30, 2023

Choose a reason for hiding this comment

Uh oh!

Nerixyz commented Apr 8, 2024

Uh oh!

This comment was marked as resolved.

Emilgardis commented Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Emilgardis commented Apr 9, 2024

Uh oh!

Uh oh!

Emilgardis Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

Nerixyz Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Nerixyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Emilgardis commented Apr 15, 2024

Uh oh!

Nerixyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Emilgardis commented Apr 30, 2023 •

edited

Loading

Emilgardis Apr 30, 2023 •

edited

Loading

Emilgardis commented Apr 9, 2024 •

edited

Loading