Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Refactor Scaleset Scaling Params - Change Size to MaxSize #2227

Closed
nharper285 opened this issue Aug 5, 2022 · 4 comments · Fixed by #2293
Closed

Refactor Scaleset Scaling Params - Change Size to MaxSize #2227

nharper285 opened this issue Aug 5, 2022 · 4 comments · Fixed by #2293
Assignees
Labels
enhancement New feature or request

Comments

@nharper285
Copy link
Contributor

nharper285 commented Aug 5, 2022

Simple request to update the current CLI commands and parameters for scaleset create. Currently size is set to both default and MaxSize. We propose adding a default parameter and setting the meaning of size to MaxSize.

Current:
auto_scale = requests.AutoScaleOptions(
min=min_instances,
max=size,
default=size,
scale_out_amount=scale_out_amount,
scale_out_cooldown=scale_out_cooldown,
scale_in_amount=scale_in_amount,
scale_in_cooldown=scale_in_cooldown,
)

Proposed:
auto_scale = requests.AutoScaleOptions(
min=min_instances,
max=size,
default=default_instances (new flag),
scale_out_amount=scale_out_amount,
scale_out_cooldown=scale_out_cooldown,
scale_in_amount=scale_in_amount,
scale_in_cooldown=scale_in_cooldown,
)

@nharper285 nharper285 added the enhancement New feature or request label Aug 5, 2022
@ghost ghost added the Needs: triage label Aug 5, 2022
@mgreisen
Copy link
Contributor

mgreisen commented Aug 5, 2022

Should we rename size to maxsize? Perhaps size is used in other contexts?

@nharper285
Copy link
Contributor Author

Should we rename size to maxsize? Perhaps size is used in other contexts?

Sure!

@tevoinea
Copy link
Member

tevoinea commented Aug 8, 2022

Do you mean renaming size the CLI argument? Or size the property on Scaleset? The former should be no issue, the latter is mapped in the ORM so we would need to be careful performing that rename.

Another would would be to keep the mapping from CLI.size to AutoScaleOptions.default and introduce a new CLI.max_size -> AutoScaleOptions.max mapping.

@mgreisen
Copy link
Contributor

mgreisen commented Aug 8, 2022

My thought was to rename the size argument in the CLI for greater clarity. We want the AutoScaleOptions.default to be a small number, preferably 1, to keep the number of nodes "spun up" when creating the scale set to a minimum. Otherwise, we wind up with a bunch of nodes spun up that then are deleted (via the scale-in rules).

@ghost ghost locked as resolved and limited conversation to collaborators Sep 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants