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

[Serve] Make serialized_policy_def and policy private #43291

Merged

Conversation

GeneDer
Copy link
Contributor

@GeneDer GeneDer commented Feb 20, 2024

Why are these changes needed?

Make serialized_policy_def and policy private so users know not to use them.

Related issue number

Closes: #43292

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@GeneDer GeneDer changed the title [Serve] Make serialized_policy_def and policy private [Serve] Make serialized_policy_def and policy private Feb 20, 2024
@GeneDer GeneDer force-pushed the make-policy-private-on-autoscaling-config branch from a9384b9 to 3648b2c Compare February 21, 2024 21:59
@@ -92,13 +93,18 @@ def replicas_settings_valid(cls, max_replicas, values):

return max_replicas

@validator("policy", always=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Validator is unable to run for attributes starting with underscore. So overriding init to call serialize_policy after init

@GeneDer GeneDer requested a review from edoakes February 22, 2024 17:12
@GeneDer GeneDer marked this pull request as ready for review February 22, 2024 17:13

# Custom autoscaling config. Defaults to the request-based autoscaler.
policy: Union[str, Callable] = DEFAULT_AUTOSCALING_POLICY
_policy: Union[str, Callable] = PrivateAttr(default=DEFAULT_AUTOSCALING_POLICY)
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't show up in the API reference because it's underscore prefixed right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@edoakes edoakes merged commit 4ae8128 into ray-project:master Feb 22, 2024
9 checks passed
@GeneDer GeneDer deleted the make-policy-private-on-autoscaling-config branch February 22, 2024 19:09
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.

[Serve] Hide policy from AutoscalingConfig
2 participants