feat!: Upgrade rev.2 #467
feat!: Upgrade rev.2 #467
Conversation
| transient=True, | ||
| disable=bool(self.no_progress), | ||
| ) as progress: | ||
| _ = progress.add_task("Analyzing cluster...", total=None) |
There was a problem hiding this comment.
can you just do progress.add_task("Analyzing cluster...", total=None) - the wildcard is extra
There was a problem hiding this comment.
I can't remember if it was, or still is, a thing - but I think there was a linter rule regarding calling functions that return a value without assigning that value to something?
| get_mock_instance_record, | ||
| ) | ||
|
|
||
| T = TypeVar("T", bound="UpgradeScenario") |
There was a problem hiding this comment.
prob use something more descriptive than just T (you can do US or UpScen or something if you want to keep it short)
| no_progress: Optional[bool] = None, | ||
| confirm_yes: Optional[bool] = None, | ||
| ops_config: Optional[List[str]] = None, |
There was a problem hiding this comment.
do we have a list of key value pairs that are actually accepted by the service? or do we assume that the service teams are the main consumers for this (and they would know the kv pairs themselves)
There was a problem hiding this comment.
Some user kvp's are described in AIO learn documentation intended for end users. Kvp's can be provided and used both internally and externally (power uses, troubleshooting etc) for whatever reasons.
There was a problem hiding this comment.
would be nice to include these in the long help but not necessary
| cluster_name=self.resource_map.connected_cluster.cluster_name, resource_group_name=resource_group_name | ||
| ) | ||
| self.init_version_map: Dict[str, dict] = {} | ||
| self.init_version_map.update(self.targets.get_extension_versions()) |
There was a problem hiding this comment.
nit: could combine this line with the one above
| @@ -58,6 +58,7 @@ def update_cluster_extension( | |||
| cluster_name: str, | |||
| extension_name: str, | |||
| update_payload: dict, | |||
| **client_kwargs, | |||
There was a problem hiding this comment.
nit: can prob remove line 54
| return bool(self.override.config) | ||
|
|
||
|
|
||
| def get_default_table() -> Table: |
There was a problem hiding this comment.
nit: please add screenshot of examples upgrade runs
parse_kvp_nargsutil function. This should be the go forward for parsing any kvp nargs based args we support. It is superior to the carry overassemble_nargs_to_dictin every way.assemble_nargs_to_dictshould eventually be deleted.Follow ups: