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

Clean up code relating to decision_policy and forecast_policy. #3957

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

EvanKrall
Copy link
Member

@EvanKrall EvanKrall commented Sep 11, 2024

We'll need to ship a yelpsoa-configs change that stops setting forecast_policy before we ship this.

@EvanKrall EvanKrall changed the title U/krall/paasta 18298 delete forecast policy decision policy Clean up code relating to decision_policy and forecast_policy. Sep 11, 2024
@EvanKrall EvanKrall marked this pull request as ready for review September 11, 2024 19:07
@EvanKrall EvanKrall requested review from cuza, jfongatyelp and nemacysts and removed request for cuza and jfongatyelp September 11, 2024 19:07
@EvanKrall
Copy link
Member Author

I could be convinced to drop the decision_policy changes, since those are slightly riskier, and tbh we should change how bespoke autoscaling works eventually.

nemacysts
nemacysts previously approved these changes Sep 13, 2024
Copy link
Member

@nemacysts nemacysts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the decision policy changes seem pretty safe to me - imo, it's probably fine to leave them in

(and i agree that we should probably change how bespoke autoscaling works in the future - i think that even the current owners get confused by how this works now: we've had a non-zero amount of onpoint requests from these owners where they've thought something was broken since there were no HPAs :p)

:bespoke:
Allows a service author to implement their own autoscaling.
This policy results in no HPA being configured.
An external process should periodically decide how many replicas this service needs to run, and use the Paasta API to tell Paasta to scale.
See the :ref:`How to create a custom (bespoke) autoscaling method` section for details.
This is most commonly used by the Kew autoscaler.

:Anything other value:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this meant to be Any other value:?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

:bespoke:
Allows a service author to implement their own autoscaling.
This policy results in no HPA being configured.
An external process should periodically decide how many replicas this service needs to run, and use the Paasta API to tell Paasta to scale.
See the :ref:`How to create a custom (bespoke) autoscaling method` section for details.
This is most commonly used by the Kew autoscaler.

:Anything other value:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:Anything other value:
:Any other value:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants