Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: use generics in pagination #417

Merged
merged 1 commit into from
Jun 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ jobs:
integration-tests:
name: Integration Tests
runs-on: ubuntu-20.04
container: hashicorp/terraform:1.1.6
container: golang:1.18-alpine3.16
timeout-minutes: 20
steps:
- name: Install Go
run: apk add go
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to install go - 1.18 is already installed as part of the docker image.
Before it would install go1.17.

- name: Install Terraform
run: apk add terraform
Copy link
Collaborator Author

@TomerHeber TomerHeber Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installing terraform instead...

- name: Checkout code
uses: actions/checkout@v2
uses: actions/checkout@v3
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noticed some depreciation warnings. Went away when switching to v3.

- name: Run Harness tests
run: go run tests/harness.go
17 changes: 5 additions & 12 deletions client/pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,8 @@ type Pagination struct {
params map[string]string
}

// Note: all the FIXME should be uncommented when Alpine adds support for go 1.18.
// This will enable generics.

type Paginated interface {
// FIXME: Environment
Environment
Copy link
Collaborator Author

@TomerHeber TomerHeber Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding back the generics in this file...
In case more paging is added in the future to other API calls. Generics will make it very simple to add support for.

getEndpoint() string
}

Expand All @@ -31,27 +28,23 @@ func (p *Pagination) next(currentPageSize int) bool {
}

// params - additional params. may be nil.
// FIXME: func getAll[P Paginated](client *ApiClient, params map[string]string) ([]P, error) {
func getAll(client *ApiClient, params map[string]string) ([]Environment, error) {
func getAll[P Paginated](client *ApiClient, params map[string]string) ([]P, error) {
p := Pagination{
offset: 0,
params: params,
}

// FIXME: var allResults []P
var allResults []Environment
var allResults []P

for {
pageParams := p.getParams()
for k, v := range params {
pageParams[k] = v
}

// FIXME: var pageResults []P
var pageResults []Environment
var pageResults []P

// FIXME: err := client.http.Get(P{}.getEndpoint(), pageParams, &pageResults)
err := client.http.Get(Environment{}.getEndpoint(), pageParams, &pageResults)
err := client.http.Get(P{}.getEndpoint(), pageParams, &pageResults)
if err != nil {
return nil, err
}
Expand Down