Skip to content

add more location to OVH and avoid exception when location is not in the list #2123

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

Open
ali-corpo wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

add more location to OVH and avoid exception when location is not in the list #2123

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f93a7ca
add more location to OVH and avoid exception when location is not in …
ali-corpo Feb 17, 2026
ef3df6d
Update libcloud/common/ovh.py
ali-corpo Feb 17, 2026
a26c788
Remove logging for unknown OVH locations
ali-corpo Feb 17, 2026
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: 18 additions & 0 deletions libcloud/common/ovh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,24 @@
"SYD1": {"id": "SYD1", "name": "Sydney 1", "country": "AU"},
"VIN1": {"id": "VIN1", "name": "Vint Hill, Virginia 1", "country": "US"},
"WAW1": {"id": "WAW1", "name": "Warsaw 1", "country": "PL"},

"BHS": {"id": "BHS", "name": "Beauharnois, Quebec", "country": "CA"},
"CA-EAST-TOR": {"id": "CA-EAST-TOR", "name": "Toronto, Canada", "country": "CA"},
"DE": {"id": "DE", "name": "Germany", "country": "DE"},
"DE1": {"id": "DE1", "name": "Germany 1", "country": "DE"},
"EU-SOUTH-MIL": {"id": "EU-SOUTH-MIL", "name": "Milan, Italy", "country": "IT"},
"EU-WEST-PAR": {"id": "EU-WEST-PAR", "name": "Paris, France", "country": "FR"},
"GRA": {"id": "GRA", "name": "Gravelines", "country": "FR"},
"GRA9": {"id": "GRA9", "name": "Gravelines 9", "country": "FR"},
"GRA11": {"id": "GRA11", "name": "Gravelines 11", "country": "FR"},
"RBX": {"id": "RBX", "name": "Roubaix", "country": "FR"},
"RBX-A": {"id": "RBX-A", "name": "Roubaix A", "country": "FR"},
"RBX-ARCHIVE": {"id": "RBX-ARCHIVE", "name": "Roubaix Archive", "country": "FR"},
"SBG": {"id": "SBG", "name": "Strasbourg", "country": "FR"},
"SBG5": {"id": "SBG5", "name": "Strasbourg 5", "country": "FR"},
"UK": {"id": "UK", "name": "United Kingdom", "country": "UK"},
"UK1": {"id": "UK1", "name": "United Kingdom 1", "country": "UK"},
Comment on lines +80 to +81
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

Using 'UK' as a country code is inconsistent with ISO 3166-1 alpha-2 standards, which uses 'GB' for the United Kingdom. However, this appears to be an existing pattern in the codebase (see line 50 with LON1). For consistency with existing entries, this may be acceptable, but consider using 'GB' for proper ISO compliance in future additions.

Suggested change
"UK": {"id": "UK", "name": "United Kingdom", "country": "UK"},
"UK1": {"id": "UK1", "name": "United Kingdom 1", "country": "UK"},
"UK": {"id": "UK", "name": "United Kingdom", "country": "GB"},
"UK1": {"id": "UK1", "name": "United Kingdom 1", "country": "GB"},

Copilot uses AI. Check for mistakes.
"WAW": {"id": "WAW", "name": "Warsaw", "country": "PL"},
}
DEFAULT_ACCESS_RULES = [
{"method": "GET", "path": "/*"},
Expand Down
7 changes: 6 additions & 1 deletion libcloud/compute/drivers/ovh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
from libcloud.compute.types import Provider, StorageVolumeState, VolumeSnapshotState
from libcloud.compute.drivers.openstack import OpenStackKeyPair, OpenStackNodeDriver

from libcloud.libcloud.utils import logging
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The import path is incorrect and contains a duplicate 'libcloud' segment. This should be import logging (standard library) or from libcloud.utils.logging import ... if using a custom logging utility. The current path from libcloud.libcloud.utils import logging will cause an ImportError at runtime.

Suggested change
from libcloud.libcloud.utils import logging
import logging

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@micafer micafer Feb 17, 2026

Choose a reason for hiding this comment

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

Remove logging import and calls, drivers do not call logging.



class OvhNodeDriver(NodeDriver):
"""
Expand Down Expand Up @@ -537,7 +539,10 @@ def _to_volumes(self, objs):
return [self._to_volume(obj) for obj in objs]

def _to_location(self, obj):
location = self.connectionCls.LOCATIONS[obj]
location = self.connectionCls.LOCATIONS.get(obj)
if not location:
logging.warning(f"Unknown location {obj} for OVH")
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

After fixing the import statement, you should create a module-level logger instance (e.g., logger = logging.getLogger(__name__)) and use logger.warning() instead of calling logging.warning() directly. This follows the established pattern in the codebase (see libcloud/compute/drivers/vsphere.py:53 and libcloud/common/google.py:103).

Copilot uses AI. Check for mistakes.
location = {"id": obj, "name": obj, "country": ""}
Comment on lines +539 to +541
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The new fallback logic for unknown locations lacks test coverage. Consider adding a test case that verifies the behavior when an unknown location is encountered, ensuring that it creates a NodeLocation with the location ID as both id and name, an empty country field, and logs an appropriate warning message.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@micafer micafer Feb 17, 2026

Choose a reason for hiding this comment

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

What's the logic behind the location being empty?
May be raising an error will be better...


return NodeLocation(driver=self, **location)

Expand Down
Loading