-
Notifications
You must be signed in to change notification settings - Fork 231
Check the installed verson when extension has patch version #4432
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3340,6 +3340,9 @@ def delete_share(self) -> None: | |||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| class AzureExtension(AzureFeatureMixin, Feature): | ||||||||||||||||||||||||||||||||||||||
| RESOURCE_NOT_FOUND = re.compile(r"ResourceNotFound", re.M) | ||||||||||||||||||||||||||||||||||||||
| _TYPE_HANDLER_VERSION_PATTERN = re.compile( | ||||||||||||||||||||||||||||||||||||||
| r"^(?P<major>\d+)\.(?P<minor>\d+)(?:\.(?P<patch>\d+))?$" | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||||||
| def create_setting( | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -3361,6 +3364,54 @@ def get( | |||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| return extension | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def normalize_type_handler_version(self, version: str) -> Tuple[str, bool]: | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| Normalize a requested extension version for installation. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||||||
| Tuple[str, bool]: | ||||||||||||||||||||||||||||||||||||||
| - normalized Major.Minor version for installation | ||||||||||||||||||||||||||||||||||||||
| - True if the original version was Major.Minor.Patch | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| requested_version = version.strip() | ||||||||||||||||||||||||||||||||||||||
| matched = self._TYPE_HANDLER_VERSION_PATTERN.fullmatch(requested_version) | ||||||||||||||||||||||||||||||||||||||
| if not matched: | ||||||||||||||||||||||||||||||||||||||
| raise LisaException( | ||||||||||||||||||||||||||||||||||||||
| "Invalid extension_version format. Expected 'Major.Minor' " | ||||||||||||||||||||||||||||||||||||||
| f"or 'Major.Minor.Patch', got '{version}'." | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3380
to
+3381
|
||||||||||||||||||||||||||||||||||||||
| "Invalid extension_version format. Expected 'Major.Minor' " | |
| f"or 'Major.Minor.Patch', got '{version}'." | |
| "Invalid extension_version format after trimming whitespace. " | |
| "Expected 'Major.Minor' or 'Major.Minor.Patch', " | |
| f"got '{requested_version}'. Verify the runbook value does not " | |
| "contain extra characters and uses only numeric version parts." |
Copilot
AI
Apr 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_installed_type_handler_version() falls back to returning the string "unknown" when it can't find a version field. That can allow callers (especially the generic test when is_patch_version is false) to proceed without actually having an installed version, masking API/SDK issues. Prefer raising a LisaException when the installed version can't be determined (and include what fields were missing) so failures are explicit and actionable.
| return str( | |
| getattr(extension_obj, "type_handler_version_name", "unknown") | |
| version = getattr(extension_obj, "type_handler_version_name", None) | |
| if version: | |
| return str(version) | |
| missing_fields = [ | |
| "instance_view.type_handler_version", | |
| "type_handler_version", | |
| "type_handler_version_name", | |
| ] | |
| raise LisaException( | |
| f"Unable to determine installed type handler version for extension " | |
| f"'{name}'. Checked fields {missing_fields}, but none were present " | |
| f"or populated in the Azure extension response. instance_view " | |
| f"{'was present' if instance_view is not None else 'was missing'}. " | |
| f"Verify the extension exists and inspect the Azure API/SDK " | |
| f"response for the extension instance view." |
Copilot
AI
Apr 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert_installed_type_handler_version() is newly added but appears unused in the repo (no references found). To reduce dead code and keep a single source of truth for the mismatch message/behavior, either use this helper from the test(s) or remove it until there’s a concrete caller.
| def assert_installed_type_handler_version( | |
| self, name: str, expected_version: str | |
| ) -> str: | |
| actual_version = self.get_installed_type_handler_version(name) | |
| if actual_version != expected_version: | |
| raise LisaException( | |
| f"Installed extension '{name}' version mismatch: " | |
| f"expected '{expected_version}', actual '{actual_version}'." | |
| ) | |
| return actual_version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extension_nameis built from the rawversion, butnormalize_type_handler_version()strips/validates a sanitized version separately. If the runbook value has leading/trailing whitespace, the extension name can become invalid even though the normalized install version is fine. Consider stripping/validatingversiononce up front, then use the sanitized value for bothextension_nameand later comparisons/logging.