Fix dropshot-pagination _stream client codegen

[jgallagher]

Closes #755; see it for details!

[jgallagher]

This is the change I'm least sure about, but I think it's fine? We should only be here if we have a non-None dropshot_pagination, and dropshot_pagination_data explicitly checks for both "page_token" and "limit" as query parameters.

[ahl]

Yeah, this is the part I'm least sure about as well. My intention (unexpressed in any comments) is that the consumer might specify the size of the first page to optimize for latency and then rely on the server to give us chunky, throughput oriented pages subsequently. Imagine, for example, a CLI where we want to start printing output quickly.

Alternatively, would it make sense to have parameters for the first page and other pages?

In either case, if we don't include this change we might consider updating the function parameters docs which, currently, come from dropshot via the OpenAPI doc.

[jgallagher]

Alternatively, would it make sense to have parameters for the first page and other pages?

I thought about this. It doesn't seem unreasonable to me, although I suspect most clients ought to specify Some(small_value) for the first page and None (server choice) for the other pages, right? I wonder if it would help to put this in a new type specifically for these parameters for clarity? Totally spitballing, something like:

pub struct StreamPaginationLimits {
    // or just omit this one and recommend clients use `.take(_)`
    pub total: Option<NonZeroU32>,
    pub first_page: Option<NonZeroU32>,
    pub remaining_pages: Option<NonZeroU32>,
}

with helper methods for "let the server pick all the limits, I don't care" and "I just want a small first page, let the server pick for remaining pages".

[jgallagher]

Another thought: I'm not sure the stream interface actually does the "small first page, larger subsequent pages" thing you want, although I guess it depends on the client. I think if you're using

while let Some(item) = stream.next() { .. }

with a first page limit of 10 and a subsequent page limit of 100, you'll incur the latency for the GET (10) on the first nex(), the next 9 will return immediately, then you'll incur the latency for GET (100) on the 11th call to next(), right?

[ahl]

Progenitor also emits the CLI. My intention with the --limit parameter for various list subcommands is that the limit impacts the total number of items output--not just the particular page size.

I expect we'd want to honor the limit by generating a take there (i.e. rather than in the obviously wrong position in the generated stream function).

Would it be superfluous to update the docs for these generated _stream() / .stream() functions to point consumers to .take() as a way to impose an outer limit or should that be implicit in the use of the stream (not sure why that didn't occur to me but FWIW it didn't).

[ahl]

Yeah, this is the part I'm least sure about as well. My intention (unexpressed in any comments) is that the consumer might specify the size of the first page to optimize for latency and then rely on the server to give us chunky, throughput oriented pages subsequently. Imagine, for example, a CLI where we want to start printing output quickly.

Alternatively, would it make sense to have parameters for the first page and other pages?

In either case, if we don't include this change we might consider updating the function parameters docs which, currently, come from dropshot via the OpenAPI doc.

[ahl]

there's similar code in method::builder_struct -- can you please update that similarly?

[jgallagher]

there's similar code in method::builder_struct -- can you please update that similarly?

Done in faf3f72

[ahl]

Looks good. Thanks