[Draft] Add pagination client support to project and violations endpoint #25
Conversation
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if len(req) == 0 { |
There was a problem hiding this comment.
The number of total items will not necessarily be a multiple of 100, which means the 'last page' will have less than 100 results.
If we change this check to len(req) < 100 (and move some of the logic around) I think we can avoid an extra request to the API in most cases.
| // GetAllViolations returns violations for the entire portfolio. Suppressed | ||
| // violations are omitted unless suppressed is true | ||
| func (c *Client) GetAllViolations(suppressed bool) ([]*PolicyViolation, error) { | ||
| // dependency track per default only returns a 100 items per get, therefore we need to iterate over all PolicyViolation pages to get all PolicyViolation | ||
|
|
||
| // all PolicyViolation found in pagination | ||
| allPolicyViolations := []*PolicyViolation{} | ||
| // the last project found in the last pagination page result | ||
| lastPaginationPage := 1 | ||
| // state var to show if all PolicyViolation projects where found | ||
| foundAll := false |
There was a problem hiding this comment.
| // GetAllViolations returns violations for the entire portfolio. Suppressed | |
| // violations are omitted unless suppressed is true | |
| func (c *Client) GetAllViolations(suppressed bool) ([]*PolicyViolation, error) { | |
| // dependency track per default only returns a 100 items per get, therefore we need to iterate over all PolicyViolation pages to get all PolicyViolation | |
| // all PolicyViolation found in pagination | |
| allPolicyViolations := []*PolicyViolation{} | |
| // the last project found in the last pagination page result | |
| lastPaginationPage := 1 | |
| // state var to show if all PolicyViolation projects where found | |
| foundAll := false | |
| // GetAllViolations returns violations for the entire portfolio. Suppressed | |
| // violations are omitted unless suppressed is true | |
| func (c *Client) GetAllViolations(suppressed bool) ([]*PolicyViolation, error) { | |
| allPolicyViolations := []*PolicyViolation{} | |
| lastPaginationPage := 1 | |
| foundAll := false |
nit: I don't think the comments are necessary. I think it's clear enough from the code.
| // GetAllProjects returns a list of all projects with the help of pagination | ||
| func (c *Client) GetAllProjects() ([]*Project, error) { | ||
| // dependency track per default only returns a 100 items per get, therefore we need to iterate over allProjects pages to get allProjects projects | ||
|
|
||
| // allProjects found in pagination | ||
| allProjects := []*Project{} | ||
| // the last project found in the last pagination page result | ||
| lastPaginationPage := 1 | ||
| // state var to show if allProjects projects where found | ||
| foundAll := false |
There was a problem hiding this comment.
| // GetAllProjects returns a list of all projects with the help of pagination | |
| func (c *Client) GetAllProjects() ([]*Project, error) { | |
| // dependency track per default only returns a 100 items per get, therefore we need to iterate over allProjects pages to get allProjects projects | |
| // allProjects found in pagination | |
| allProjects := []*Project{} | |
| // the last project found in the last pagination page result | |
| lastPaginationPage := 1 | |
| // state var to show if allProjects projects where found | |
| foundAll := false | |
| // GetAllProjects returns a list of all projects | |
| func (c *Client) GetAllProjects() ([]*Project, error) { | |
| allProjects := []*Project{} | |
| lastPaginationPage := 1 | |
| foundAll := false |
nit: I don't think the comments are necessarily. It's clear enough from the code.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if len(req) == 0 { |
There was a problem hiding this comment.
The number of total items will not necessarily be a multiple of 100, which means the 'last page' will have less than 100 results.
If we change this check to len(req) < 100 (and move some of the logic around) I think we can avoid an extra request to the API in most cases.
|
I've opened an issue for the request duration: #30 |
|
@ribbybibby great, thanks for you feedback, really good stuff. Will implement these next week. |
|
Pagination came along with #33, so I'm going to close this PR. Thank you very much for the time you spent on it though, @jakoberpf. I'm going to cut a 0.2.0 release with these changes soon. When that drops it would be great if you could upgrade and provide some stats on #30 in terms of the request duration you see to the exporter and the number of projects and violations you have in your DT instance. I'm really interested in making this exporter as usable and performant as possible but I don't personally run a very large DT instance, so your insight would be very valuable. |
|
@ribbybibby Oh great, sorry that I didn't came around to make the changes before. I will post some metrics with the new version, when is there :-) I am on holiday til mid Jan, from next week, though. |
Hello Everyone,
this PR wants to add support for pagination pulling from DT, as its endpoint only return a max of a 100 items per request. Therefore if you have more than a 100 projects or violations, your metrics result will be incomplete.
There are two endpoints used in the exporter which are relevant for this
api/v1/projectsandapi/v1/violations. In this PR we add an iterative logic to the client, where it will iterates over increasing pagination pages until there are not items returned any more. This way we can get all items from a resource.The major downside which we see right now and a looking to fix is the request duration when (like in our case) you have about 60.000 violations spread about a hundrets of projects. Goal would be to make pulling the violations optional from the
/metrics?pullViolations=booleanendpoint.Any feedback would be appreciated