The option generator goroutine and the task fetcher goroutine are more
readable when extracted into another method. Also, added some commenting
documenting what their expectations are
This ensures that setup time does not impact the results of the
benchmark since profiling showed that much time was spent setting up the
test array of tasks. This also uses `make` to build that slice instead.
Also, the tests revealed that there was a bug where offsets were
pre-incremented rather than post-incremented, omitting the first 100
results.
Because we were testing the mock client, and not the paginating
kapacitor client for the case where limit and offset were provided, an
issue with that code path was not exposed.
The issues exposed were that the condition was incorrect for triggering
this behavior, and no return clause was present to prevent the remainder
of the ListTasks method from running.
Kapacitor responses are paginated, and sometimes users have more than
the default 100 tasks that are returned from Kapacitor. This replaces
the previous Kapa client with one that automatically follows paginated
responses from Kapacitor's ListTasks endpoint and returns the full
response.
Tests for the KapacitorRulesGet endpoint had to be updated because they
did not account for "limit" and "offset", and so led to an infinite
loop with the paginated client. A correct kapacitor backend will not
have this behavior
The kapacitor client used in the kapacitor endpoints is limited to
fetching whatever limit you provide it. If you provide no limit, it
defaults to a limit of 100. We use this default behavior currently.
Some users have more than 100 tasks, so we need a client that's capable
of continually fetching tasks from Kapacitor until there are none left,
and returning the full response to the frontend.
This introduces a PaginatingKapacitorClient which does exactly that.
Also, test coverage was added around the KapacitorRulesGet endpoint,
since it was previously untested.
Because we are now creating new instances of dashboards when we create a
response, it's critical to copy every element of Dashboards from the
previous to the new instance.
We were not previously copying the Type field of cells, so this was
defaulting to the empty string zero value. This patch adds "Type" to the
tests and ensures that it's properly copied
Tim Raymond
Andrew Watkins
Hunter Trujillo
Timothy J. Raymond
Jared Scheib