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.

Sign up for GitHub

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

{Compute Diagnostic} Update required args for spot-placement-recommender cli #30338

Merged
merged 3 commits into from
Nov 15, 2024
Merged

{Compute Diagnostic} Update required args for spot-placement-recommender cli #30338

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c01574e
change compute-recommender from GA to preview
joal42 Nov 1, 2024
3cac4fa
Update the required args for spot-recommender
joal42 Nov 13, 2024
caa830f
Merge remote-tracking branch 'origin/dev' into feature/joal/update-co…
joal42 Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions ...modules/compute_recommender/aaz/latest/compute_recommender/_spot_placement_recommender.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ def _build_arguments_schema(cls, *args, **kwargs):

_args_schema = cls._args_schema
_args_schema.location = AAZResourceLocationArg(
required=True
help="the Azure region where the recommendation will be generated",
required=True,
is_preview=True,
id_part="name",
)

# define Arg Group "SpotPlacementScoresInput"
Expand All @@ -56,21 +59,28 @@ def _build_arguments_schema(cls, *args, **kwargs):
options=["--availability-zones"],
arg_group="SpotPlacementScoresInput",
help="Defines if the scope is zonal or regional.",
is_preview=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, labeling GA commands as is_preview is a recommended behavior. Is this because the previous PR forgot to mark it as preview?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. the cli is in public preview stage. So, marking the parameters to public preview too.

)
_args_schema.desired_count = AAZIntArg(
options=["--desired-count"],
arg_group="SpotPlacementScoresInput",
help="Desired instance count per region/zone based on the scope.",
required=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that required=True will cause breaking changes to the customer's automated scripts.
Due to the late submission of this PR, you have missed the train of breaking change window for this sprint. I suggest releasing the breaking change part in the next breaking change window (next year's build event sprint)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any choice to release it earlier before breaking change release? It is to correct the optional/required parameter. Today, if customers miss those parameter, they will get error from our api. I don't think this change will break the customer's script.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, got it. It's not considered a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ joal42 It will be released in the next sprint (12-10)

is_preview=True,
)
_args_schema.desired_locations = AAZListArg(
options=["--desired-locations"],
arg_group="SpotPlacementScoresInput",
help="The desired regions",
required=True,
is_preview=True,
)
_args_schema.desired_sizes = AAZListArg(
options=["--desired-sizes"],
arg_group="SpotPlacementScoresInput",
help="The desired resource SKUs.",
required=True,
is_preview=True,
)

desired_locations = cls._args_schema.desired_locations
Expand Down Expand Up @@ -173,9 +183,9 @@ def content(self):
typ_kwargs={"flags": {"required": True, "client_flatten": True}}
)
_builder.set_prop("availabilityZones", AAZBoolType, ".availability_zones")
_builder.set_prop("desiredCount", AAZIntType, ".desired_count")
_builder.set_prop("desiredLocations", AAZListType, ".desired_locations")
_builder.set_prop("desiredSizes", AAZListType, ".desired_sizes")
_builder.set_prop("desiredCount", AAZIntType, ".desired_count", typ_kwargs={"flags": {"required": True}})
_builder.set_prop("desiredLocations", AAZListType, ".desired_locations", typ_kwargs={"flags": {"required": True}})
_builder.set_prop("desiredSizes", AAZListType, ".desired_sizes", typ_kwargs={"flags": {"required": True}})

desired_locations = _builder.get(".desiredLocations")
if desired_locations is not None:
Expand Down