XAMPPRocky / octocrab

A modern, extensible GitHub API Client for Rust.
Other
1.11k stars 265 forks source link

Add a page iterator for pageable responses #152

Open mkroman opened 2 years ago

mkroman commented 2 years ago

It's currently quite cumbersome to iterate over pages on a resource.

It would be great if resources that return a list with multiple pages instead return an iterator that makes it easy to iterate over elements and have the iterator automatically fetch subsequent pages.

Instead of this:

    pub async fn org_hooks(&self) -> Result<Vec<Hook>, Error> {
        let mut current_page: Page<Hook> = self
            .octocrab
            .get(
                format!("/orgs/{}/hooks", self.config.organization),
                None::<&()>,
            )
            .await?;

        let mut results = current_page
            .total_count
            .map_or_else(Vec::new, |n| Vec::with_capacity(n as usize));

        results.extend(current_page.take_items());

        while let Some(mut page) = self.octocrab.get_page::<Hook>(&current_page.next).await? {
            results.extend(page.take_items());
            current_page = page;
        }

        Ok(results)
    }

I imagine something like this:

    pub async fn org_hooks(&self) -> Result<Vec<Hook>, Error> {
        let mut hooks = self.octocrab
            .get_iter(
                format!("/orgs/{}/hooks", self.config.organization),
                None::<&()>,
            )
            .await?;

        let result = hooks.collect::<Vec<Hook>>.await?;

        Ok(results)
    }
XAMPPRocky commented 2 years ago

Thank you for your issue! I think this has already been resolved with Octocrab::all_pages if you want to eagerly get all items, it hasn't been released yet, but I intend to make a release soon. https://github.com/XAMPPRocky/octocrab/blob/master/src/lib.rs#L875

Yes, I'd ideally like to make Page<T> an iterator, however we're still a long ways off from async iterators being stable, which is what we'd require to give a nice API.

mkroman commented 2 years ago

Ah, I wasn't aware of all_pages. That helps a bit - thanks!

however we're still a long ways off from async iterators being stable,

True, but even something like while let will go a long way for certain things :slightly_smiling_face:

NickLarsenNZ commented 3 months ago

Adding a permalink for those who are looking for the all_pages method linked above:

https://github.com/XAMPPRocky/octocrab/blob/c8910fec661e65ccc86ee7d94982ed89e6267692/src/lib.rs#L1633-L1646