Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

updated flexplugin to look for 200 volumes #753

Merged
merged 1 commit into from
Jun 14, 2018
Merged

updated flexplugin to look for 200 volumes #753

merged 1 commit into from
Jun 14, 2018

Conversation

ZachGlassman
Copy link
Contributor

@ZachGlassman ZachGlassman commented May 5, 2018 •
edited

Updates the getVolumeByName function to request 200 volumes per name.

Also fixes a type error in the provision.go code.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 5, 2018
@ZachGlassman
Copy link
Contributor Author

@klausenbusk can you have a look here?

@klausenbusk
Copy link
Contributor

Could you explain why this is needed?

@klausenbusk
Copy link
Contributor

/area digitalocean

@ZachGlassman
Copy link
Contributor Author

We have found this is a bit more resiliant to API failures in the paging system. In addition, the Digital Ocean API has a rate limiter and so this can help reduce the number of requests needed when working with large numbers of volumes.

As for the change in provision.go, I'm guessing this was due to an update, but this (or some other change) seems necessary to compile (at least on my machine).

@klausenbusk
Copy link
Contributor

We have found this is a bit more resiliant to API failures in the paging system. In addition, the Digital Ocean API has a rate limiter and so this can help reduce the number of requests needed when working with large numbers of volumes.

As for the change in provision.go, I'm guessing this was due to an update, but this (or some other change) seems necessary to compile (at least on my machine).

Could you update the commit message to include this? Besides that it LGTM :)

@ZachGlassman
updated flexplugin to look for 200 volumes
d34ecfa 
This makes the system a bit more resiliant to API failures affecting listing volumes

Additionally, this helps limit the number of API requests necessary when working with systems containing large volumes.

Change in provision.go in response to other update
@klausenbusk
Copy link
Contributor

LGTM :)

As for the change in provision.go, I'm guessing this was due to an update, but this (or some other change) seems necessary to compile (at least on my machine).

It was changed in 1.10: kubernetes/kubernetes#56460

@wongma7
Copy link
Contributor

wongma7 commented Jun 14, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 14, 2018
@wongma7 wongma7 merged commit 4f006fa into kubernetes-retired:master Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@wongma7 wongma7

Labels
area/digitalocean cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ZachGlassman @klausenbusk @wongma7 @k8s-ci-robot