feat: add support for GetProjectBatch and GetVersionBatch#73
Conversation
The API for both endpoint is paginated, so this implementation return an iterator that allow the library user to controle how he wants to consume the response.
|
Hi @zaibon ! thanks so much for the PR. |
| NextPageToken string `json:"nextPageToken"` | ||
| } | ||
|
|
||
| func getProjectBatch(ctx context.Context, c *client.Client, projectNames []string) <-chan batchJob[def.Project] { |
There was a problem hiding this comment.
the 2 methodsgetProjectBatch and getVersionBatch are very similar. I wonder if there is a way to refactor them into a single function...
There was a problem hiding this comment.
Found a way to extract the duplicate logic.
If you found this becoming too much magic, I can drop the last commit f8e0bd0
|
@edoardottt I addressed the linting errors. CI is happy now 😄 |
|
Thank u so much @zaibon. I'm gonna review the PR. Thanks again :) |
|
At the moment, my implantation does not support requesting a specific page. It only exposes an easy API to walk over all the pages in order. The API itself does not really make it easy to request a specific page because it does not use a number of pages in the request. It uses a token based approach. So you only know the next token after seeing the first page. |
edoardottt
left a comment
edoardottt
left a comment
There was a problem hiding this comment.
Nice job!
Left some thougths, waiting to read what you think about :)
|
|
||
| func TestGetProjectBatch(t *testing.T) { | ||
| t.Run("GetProject batch", func(t *testing.T) { | ||
| iter, err := api.GetProjectBatch([]string{ |
There was a problem hiding this comment.
This input returns a response with 3 items in a single page. Can we have a test checking for correct results in paginated results too?
We don't have to check the responses' content, just if the items count match.
There was a problem hiding this comment.
Added a test that returns 300 results.
| results, err := consumeIter(iter) | ||
| require.NoError(t, err) | ||
|
|
||
| assert.Equal(t, expected, results) |
There was a problem hiding this comment.
Like previously in other discussions, I don't know if this is the best way to check the correctness of code.
E.g. I could release a new version of defangjs and this test case will fail :/
There was a problem hiding this comment.
No it will not since we're requesting details of a specific version. The logic is the same used in the TestGetVersion test.
|
Thanks so much @zaibon ! |
2 participants
The API for both endpoint is paginated, so this implementation return an iterator that allow the library user to controle how he wants to consume the response.