Copilot commented on code in PR #2125:
URL: https://github.com/apache/libcloud/pull/2125#discussion_r2816600905
##########
libcloud/compute/drivers/ovh.py:
##########
@@ -585,7 +600,7 @@ def _to_sizes(self, objs):
return [self._to_size(obj) for obj in objs]
def _to_image(self, obj):
- extra = {"region": obj["region"], "visibility": obj["visibility"]}
+ extra = {"region": obj["region"], "visibility": obj["visibility"],
"status": obj["status"]}
Review Comment:
`_to_image` now unconditionally reads `obj["status"]`, but the OVH image
list/details responses used elsewhere in the driver (and in existing fixtures)
don't include a `status` field. This will raise a `KeyError` when calling
`list_images()` / `get_image()`. Use `obj.get("status")` (or a sensible
default) and keep the extra fields optional.
```suggestion
extra = {}
for key in ("region", "visibility", "status"):
if key in obj:
extra[key] = obj[key]
```
##########
libcloud/compute/drivers/ovh.py:
##########
@@ -631,3 +646,16 @@ def _to_snapshots(self, objs):
def _ex_connection_class_kwargs(self):
return {"ex_consumer_key": self.consumer_key, "region": self.region}
+
+ def create_image(self, node, name, description=None):
+ action = self._get_project_action("instance/%s/snapshot" % node.id)
+ response = self.connection.request(action, method="POST",
data={"snapshotName": name})
Review Comment:
Trailing whitespace at the end of this line should be removed to keep the
file clean and avoid style/lint noise.
##########
libcloud/compute/drivers/ovh.py:
##########
@@ -210,7 +211,21 @@ def list_images(self, location=None, ex_size=None):
params["flavorId"] = ex_size.id
response = self.connection.request(action, params=params)
- return self._to_images(response.object)
+ public_images = self._to_images(response.object)
+
+ action_snapshot = self._get_project_action("snapshot")
+ params_snapshot = {}
+
+ if location:
+ params_snapshot["region"] = location.id
+
+ # if ex_size:
+ # params["flavorType"] = ex_size.
+ response_snapshot = self.connection.request(action_snapshot,
params=params_snapshot)
+ private_images = self._to_images(response_snapshot.object)
Review Comment:
`list_images()` now makes an additional request to the `snapshot` endpoint,
but the existing OVH compute unit tests/mock transport don't provide a
handler/fixture for that URL. This change will make
`libcloud/test/compute/test_ovh.py::test_list_images` fail unless the mock and
fixtures are updated (and ideally assertions added for snapshot images).
```suggestion
try:
action_snapshot = self._get_project_action("snapshot")
params_snapshot = {}
if location:
params_snapshot["region"] = location.id
# if ex_size:
# params["flavorType"] = ex_size.
response_snapshot = self.connection.request(
action_snapshot, params=params_snapshot
)
private_images = self._to_images(response_snapshot.object)
except Exception:
# If the snapshot endpoint is not available or not mocked,
# gracefully fall back to no private images.
private_images = []
```
##########
libcloud/compute/drivers/ovh.py:
##########
@@ -210,7 +211,21 @@ def list_images(self, location=None, ex_size=None):
params["flavorId"] = ex_size.id
response = self.connection.request(action, params=params)
- return self._to_images(response.object)
+ public_images = self._to_images(response.object)
+
+ action_snapshot = self._get_project_action("snapshot")
+ params_snapshot = {}
+
+ if location:
+ params_snapshot["region"] = location.id
+
+ # if ex_size:
+ # params["flavorType"] = ex_size.
Review Comment:
There is commented-out/incomplete code left in this method (`# if ex_size:`
/ `params["flavorType"] = ex_size.`). Since it can't work as-is and doesn't
document an implemented behavior, it should be removed or replaced with a
proper implementation and explanation.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
