Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scaling policy type does not support all its possible values #55

Closed
james64 opened this issue Mar 3, 2021 · 2 comments · Fixed by #65
Closed

Scaling policy type does not support all its possible values #55

james64 opened this issue Mar 3, 2021 · 2 comments · Fixed by #65
Labels
bug 🐛 An issue with the system

Comments

@james64
Copy link

james64 commented Mar 3, 2021

Describe the Bug

Module has variables scale_up_policy_type and scale_down_policy_type. According to documentation they support all values allowed in autoscaling_policy tf resource namely SimpleScaling, StepScaling and TargetTrackingScaling. On the other hand implementation of scaling policies does not contain blocks step_adjustments and target_tracking_configuration which are necessary to use StepScaling or TargetTrackingScaling respectively. This means that effectively only SimpleScaling is available.

Expected Behavior

Option A - support SimpleScaling

  • Drop variables scale_*_policy_type and replace them with hardcoded SimpleScaling
  • Explicit description of this in readme

Option B - full support for all three policies

@james64 james64 added the bug 🐛 An issue with the system label Mar 3, 2021
@nitrocode
Copy link
Member

That makes sense.

Option B makes the most sense to me. If you have some free time, please feel free to contribute 🙏

@Nuru
Copy link
Sponsor Contributor

Nuru commented Apr 28, 2021

@james64 Thank you for bringing this to our attention.

For now, we are going to document that only SimpleScaling is supported, but we will leave the variables (with default values) to leave room for future enhancements and to not break existing modules that set them.

We will accept a PR that adds support for the other scaling types if it includes examples of them in the examples/complete test.

Nuru added a commit that referenced this issue Apr 29, 2021
@Nuru Nuru closed this as completed in #65 Apr 30, 2021
Nuru added a commit that referenced this issue Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An issue with the system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants