-
Notifications
You must be signed in to change notification settings - Fork 244
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
Add mechanism to list more than 100 incidents #429
base: main
Are you sure you want to change the base?
Conversation
By default we can reach only 100 elements from a given range, if we have more than 100 elements between `Since` and `Until`, we need to use an offeset to get the 101st element and so on.
I will implement it for v1.4.2. If you think is good for the project, I will reopen it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this is a new feature it would not be released as part of v1.4.x
or v1.5.x
, and could be included in v1.6.0
. I think there's value in adding it, but I do have some minor requested changes before it could be merged. 👍
@@ -183,6 +188,37 @@ func (c *Client) ListIncidentsWithContext(ctx context.Context, o ListIncidentsOp | |||
return &result, nil | |||
} | |||
|
|||
// ListIncidentsPaginated lists existing services processing paginated responses | |||
func (c *Client) ListIncidentsPaginatedWithContext(ctx context.Context, o ListIncidentsOptions) ([]Incident, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this method signature aligns with future plans of the project. The WithContext
should be removed from the name, as this is a new method and we have no reason to support a non-context APIs for new things. Also, it should accept an include function that allows filtering of unwanted items.
@@ -163,6 +164,10 @@ func (c *Client) ListIncidents(o ListIncidentsOptions) (*ListIncidentsResponse, | |||
return c.ListIncidentsWithContext(context.Background(), o) | |||
} | |||
|
|||
func (c *Client) ListIncidentsPaginated(o ListIncidentsOptions) ([]Incident, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method should be removed, as we don't want to introduce new methods that don't accept a context.Contex
.
By default we can reach only 100 elements from a given range, if we have more than 100 elements
between
Since
andUntil
, we need to use an offeset to get the 101st element and so on.