Skip to content

Commit 971e960

Browse files
joaopapereirajoaopapereira
authored
[main] Make retrieval of tasks in bulk instead of small increments (#3275)
* Make retrieval of tasks in bulk instead of small increments Signed-off-by: João Pereira <[email protected]> * Address PR comments - Fix issue, where code was always doing 2 requests even when it was able to get all the tasks in the first request Signed-off-by: João Pereira <[email protected]> * Fix lack of output Signed-off-by: João Pereira <[email protected]> * Remove extra tasks from first request Signed-off-by: João Pereira <[email protected]> * Hardcode version because no tag exists for main branch Signed-off-by: João Pereira <[email protected]> * Change approach by providing the query parameters always to the request Signed-off-by: João Pereira <[email protected]> --------- Signed-off-by: João Pereira <[email protected]>
1 parent dd2e812 commit 971e960

File tree

5 files changed

+186
-12
lines changed

5 files changed

+186
-12
lines changed

‎Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ FLAKE_ATTEMPTS ?=5
44
PACKAGES ?= api actor command types util version integration/helpers
55
LC_ALL = "en_US.UTF-8"
66

7-
CF_BUILD_VERSION ?= $$(git describe --tags --abbrev=0)
7+
## TODO: Change when new version is released
8+
CF_BUILD_VERSION ?= v9.0.0
9+
#CF_BUILD_VERSION ?= $$(git describe --tags --abbrev=0)
810
CF_BUILD_SHA ?= $$(git rev-parse --short HEAD)
911
CF_BUILD_DATE ?= $$(date -u +"%Y-%m-%d")
1012
LD_FLAGS_COMMON=-w -s \

‎api/cloudcontroller/ccv3/included_resources.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,14 @@ type IncludedResources struct {
1212
ServicePlans []resources.ServicePlan `json:"service_plans,omitempty"`
1313
Apps []resources.Application `json:"apps,omitempty"`
1414
}
15+
16+
func (i *IncludedResources) Merge(resources IncludedResources) {
17+
i.Apps = append(i.Apps, resources.Apps...)
18+
i.Users = append(i.Users, resources.Users...)
19+
i.Organizations = append(i.Organizations, resources.Organizations...)
20+
i.Spaces = append(i.Spaces, resources.Spaces...)
21+
i.ServiceBrokers = append(i.ServiceBrokers, resources.ServiceBrokers...)
22+
i.ServiceInstances = append(i.ServiceInstances, resources.ServiceInstances...)
23+
i.ServiceOfferings = append(i.ServiceOfferings, resources.ServiceOfferings...)
24+
i.ServicePlans = append(i.ServicePlans, resources.ServicePlans...)
25+
}

‎api/cloudcontroller/ccv3/paginate.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,7 @@ func (requester RealRequester) paginate(request *cloudcontroller.Request, obj in
1717
return IncludedResources{}, fullWarningsList, err
1818
}
1919

20-
includes.Apps = append(includes.Apps, wrapper.IncludedResources.Apps...)
21-
includes.Users = append(includes.Users, wrapper.IncludedResources.Users...)
22-
includes.Organizations = append(includes.Organizations, wrapper.IncludedResources.Organizations...)
23-
includes.Spaces = append(includes.Spaces, wrapper.IncludedResources.Spaces...)
24-
includes.ServiceBrokers = append(includes.ServiceBrokers, wrapper.IncludedResources.ServiceBrokers...)
25-
includes.ServiceInstances = append(includes.ServiceInstances, wrapper.IncludedResources.ServiceInstances...)
26-
includes.ServiceOfferings = append(includes.ServiceOfferings, wrapper.IncludedResources.ServiceOfferings...)
27-
includes.ServicePlans = append(includes.ServicePlans, wrapper.IncludedResources.ServicePlans...)
20+
includes.Merge(wrapper.IncludedResources)
2821

2922
if specificPage || wrapper.NextPage() == "" {
3023
break

‎api/cloudcontroller/ccv3/task.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@ func (client *Client) CreateApplicationTask(appGUID string, task resources.Task)
2525
func (client *Client) GetApplicationTasks(appGUID string, query ...Query) ([]resources.Task, Warnings, error) {
2626
var tasks []resources.Task
2727

28+
foundPerPageQuery := false
29+
for _, keyVal := range query {
30+
if keyVal.Key == PerPage {
31+
foundPerPageQuery = true
32+
}
33+
}
34+
if !foundPerPageQuery {
35+
query = append(query, Query{Key: PerPage, Values: []string{MaxPerPage}})
36+
}
37+
2838
_, warnings, err := client.MakeListRequest(RequestParams{
2939
RequestName: internal.GetApplicationTasksRequest,
3040
URIParams: internal.Params{"app_guid": appGUID},

‎api/cloudcontroller/ccv3/task_test.go

Lines changed: 161 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,18 @@ var _ = Describe("Task", func() {
190190
warnings Warnings
191191
executeErr error
192192
)
193+
BeforeEach(func() {
194+
// This is required because ginkgo does not instantiate variable per test context so the tests pollute the
195+
// variables for the next tests.
196+
submitQuery = Query{}
197+
})
193198

194199
JustBeforeEach(func() {
195-
tasks, warnings, executeErr = client.GetApplicationTasks("some-app-guid", submitQuery)
200+
if submitQuery.Key == "" {
201+
tasks, warnings, executeErr = client.GetApplicationTasks("some-app-guid")
202+
} else {
203+
tasks, warnings, executeErr = client.GetApplicationTasks("some-app-guid", submitQuery)
204+
}
196205
})
197206

198207
When("the application exists", func() {
@@ -201,7 +210,8 @@ var _ = Describe("Task", func() {
201210
"pagination": {
202211
"next": {
203212
"href": "%s/v3/apps/some-app-guid/tasks?per_page=2&page=2"
204-
}
213+
},
214+
"total_results": 3
205215
},
206216
"resources": [
207217
{
@@ -245,7 +255,7 @@ var _ = Describe("Task", func() {
245255
)
246256
server.AppendHandlers(
247257
CombineHandlers(
248-
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "per_page=2&page=2"),
258+
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "page=2&per_page=2"),
249259
RespondWith(http.StatusOK, response2, http.Header{"X-Cf-Warnings": {"warning-2"}}),
250260
),
251261
)
@@ -353,6 +363,154 @@ var _ = Describe("Task", func() {
353363
Expect(warnings).To(ConsistOf("warning"))
354364
})
355365
})
366+
367+
When("the application has 10000 tasks", func() {
368+
BeforeEach(func() {
369+
response2 := fmt.Sprintf(`{
370+
"pagination": {
371+
"next": {
372+
"href": "%s/v3/apps/some-app-guid/tasks?per_page=5000&page=2"
373+
},
374+
"total_results": 10000
375+
},
376+
"resources": [
377+
{
378+
"guid": "task-1-guid",
379+
"sequence_id": 1,
380+
"name": "task-1",
381+
"command": "some-command",
382+
"state": "SUCCEEDED",
383+
"created_at": "2016-11-07T05:59:01Z"
384+
},
385+
{
386+
"guid": "task-2-guid",
387+
"sequence_id": 2,
388+
"name": "task-2",
389+
"command": "some-command",
390+
"state": "FAILED",
391+
"created_at": "2016-11-07T06:59:01Z"
392+
}
393+
]
394+
}`, server.URL())
395+
response3 := fmt.Sprintf(`{
396+
"pagination": {
397+
"total_results": 10000
398+
},
399+
"resources": [
400+
{
401+
"guid": "task-1-guid",
402+
"sequence_id": 1,
403+
"name": "task-1",
404+
"command": "some-command",
405+
"state": "SUCCEEDED",
406+
"created_at": "2016-11-07T05:59:01Z"
407+
},
408+
{
409+
"guid": "task-2-guid",
410+
"sequence_id": 2,
411+
"name": "task-2",
412+
"command": "some-command",
413+
"state": "FAILED",
414+
"created_at": "2016-11-07T06:59:01Z"
415+
}
416+
]
417+
}`)
418+
419+
server.AppendHandlers(
420+
CombineHandlers(
421+
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "per_page=5000"),
422+
RespondWith(http.StatusOK, response2, http.Header{"X-Cf-Warnings": {"warning-2"}}),
423+
),
424+
)
425+
server.AppendHandlers(
426+
CombineHandlers(
427+
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "page=2&per_page=5000"),
428+
RespondWith(http.StatusOK, response3, http.Header{"X-Cf-Warnings": {"warning-2"}}),
429+
),
430+
)
431+
})
432+
433+
It("calls CAPI 2 times", func() {
434+
Expect(executeErr).ToNot(HaveOccurred())
435+
})
436+
})
437+
438+
When("the application has 4999 tasks", func() {
439+
BeforeEach(func() {
440+
response2 := fmt.Sprintf(`{
441+
"pagination": {
442+
"total_results": 4999
443+
},
444+
"resources": [
445+
{
446+
"guid": "task-1-guid",
447+
"sequence_id": 1,
448+
"name": "task-1",
449+
"command": "some-command",
450+
"state": "SUCCEEDED",
451+
"created_at": "2016-11-07T05:59:01Z"
452+
},
453+
{
454+
"guid": "task-2-guid",
455+
"sequence_id": 2,
456+
"name": "task-2",
457+
"command": "some-command",
458+
"state": "FAILED",
459+
"created_at": "2016-11-07T06:59:01Z"
460+
}
461+
]
462+
}`)
463+
server.AppendHandlers(
464+
CombineHandlers(
465+
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks", "per_page=5000"),
466+
RespondWith(http.StatusOK, response2, http.Header{"X-Cf-Warnings": {"warning-2"}}),
467+
),
468+
)
469+
})
470+
471+
It("calls CAPI 2 times", func() {
472+
Expect(executeErr).ToNot(HaveOccurred())
473+
})
474+
})
475+
476+
When("the application has 2 tasks", func() {
477+
BeforeEach(func() {
478+
response1 := fmt.Sprintf(`{
479+
"pagination": {
480+
"total_results": 2
481+
},
482+
"resources": [
483+
{
484+
"guid": "task-1-guid",
485+
"sequence_id": 1,
486+
"name": "task-1",
487+
"command": "some-command",
488+
"state": "SUCCEEDED",
489+
"created_at": "2016-11-07T05:59:01Z"
490+
},
491+
{
492+
"guid": "task-2-guid",
493+
"sequence_id": 2,
494+
"name": "task-2",
495+
"command": "some-command",
496+
"state": "FAILED",
497+
"created_at": "2016-11-07T06:59:01Z"
498+
}
499+
]
500+
}`)
501+
502+
server.AppendHandlers(
503+
CombineHandlers(
504+
VerifyRequest(http.MethodGet, "/v3/apps/some-app-guid/tasks"),
505+
RespondWith(http.StatusAccepted, response1, http.Header{"X-Cf-Warnings": {"warning"}}),
506+
),
507+
)
508+
})
509+
510+
It("calls CAPI 1 time", func() {
511+
Expect(executeErr).ToNot(HaveOccurred())
512+
})
513+
})
356514
})
357515

358516
Describe("UpdateTaskCancel", func() {

0 commit comments

Comments
 (0)