Use Go SDK Iterators when listing resources with the CLI

[mgyucht]

Changes

Currently, when the CLI run a list API call (like list jobs), it uses the List*All methods from the SDK, which list all resources in the collection. This is very slow for large collections: if you need to list all jobs from a workspace that has 10,000+ jobs, you'll be waiting for at least 100 RPCs to complete before seeing any output.

Instead of using List*All() methods, the SDK recently added an iterator data structure that allows traversing the collection without needing to completely list it first. New pages are fetched lazily if the next requested item belongs to the next page. Using the List() methods that return these iterators, the CLI can proactively print out some of the response before the complete collection has been fetched.

This involves a pretty major rewrite of the rendering logic in cmdio. The idea there is to define custom rendering logic based on the type of the provided resource. There are three renderer interfaces:

1. textRenderer: supports printing something in a textual format (i.e. not JSON, and not templated).
2. jsonRenderer: supports printing something in a pretty-printed JSON format.
3. templateRenderer: supports printing something using a text template.

There are also three renderer implementations:

1. readerRenderer: supports printing a reader. This only implements the textRenderer interface.
2. iteratorRenderer: supports printing a listing.Iterator from the Go SDK. This implements jsonRenderer and templateRenderer, buffering 20 resources at a time before writing them to the output.
3. defaultRenderer: supports printing arbitrary resources (the previous implementation).

Callers will either use cmdio.Render() for rendering individual resources or io.Reader or cmdio.RenderIterator() for rendering an iterator. This separate method is needed to safely be able to match on the type of the iterator, since Go does not allow runtime type matches on generic types with an existential type parameter.

One other change that needs to happen is to split the templates used for text representation of list resources into a header template and a row template. The template is now executed multiple times for List API calls, but the header should only be printed once. To support this, I have added headerTemplate to cmdIO, and I have also changed RenderWithTemplate to include a headerTemplate parameter everywhere.

Tests

• Unit tests for text rendering logic
• Unit test for reflection-based iterator construction.

[codecov-commenter]

Codecov Report

Attention: 81 lines in your changes are missing coverage. Please review.

Comparison is base (2683341) 52.05% compared to head (147031e) 52.43%.

Files	Patch %	Lines
libs/cmdio/render.go	54.54%	53 Missing and 17 partials ⚠️
cmd/root/io.go	0.00%	3 Missing ⚠️
cmd/fs/cp.go	0.00%	2 Missing ⚠️
cmd/fs/ls.go	0.00%	2 Missing ⚠️
cmd/workspace/repos/repos.go	0.00%	2 Missing ⚠️
cmd/fs/cat.go	0.00%	1 Missing ⚠️
internal/bundle/helpers.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1202      +/-   ##
==========================================
+ Coverage   52.05%   52.43%   +0.37%     
==========================================
  Files         308      308              
  Lines       17470    17586     +116     
==========================================
+ Hits         9094     9221     +127     
+ Misses       7706     7672      -34     
- Partials      670      693      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pietern]

Nice work, glad to see this.

[pietern]

Looking great. A thought I had while reading this: as we're leaning on reflection to pick the right rendering mode, we'll need to work on integration tests for all generated commands in all output modes. With the mocking support in the SDK, we can probably come up with something (relatively) lightweight that gets us the coverage.

[mgyucht]

As for integration tests like what you're mentioning, I think it would be a reasonable idea to have coverage like that anyways to make sure the SDK/CLI work properly for all API calls, separately from this PR. Definitely, we need to make sure this is accounted for though, as issues won't be caught at compile time anymore.