Skip to content

Add OHV datacenter #8170 #8178

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

Merged
merged 7 commits into from
Jun 3, 2025
Merged

Conversation

t-couery
Copy link
Contributor

Issue

Add OVH Data center. Request : #8170

Description

Complete the request to add data centres.
OVH is the largest hosting provider in Europe.

More information here :
https://www.ovhcloud.com/fr/datacenters-ovhcloud/

Preview

  "ovh-fr-roubaix": {
    "provider": "ovh",
    "displayName": "Roubaix",
    "zoneKey": "FR",
    "lonlat": [50.6925, 3.20422],
    "region": "Europe",
    "source": "https://www.ovhcloud.com/fr/datacenters-ovhcloud/",
    "operationalSince": null,
    "status": "operational"
  }
  • I have run pnpx prettier@2 --write . and poetry run format in the top level directory to format my changes.

"displayName": "Hillsboro",
"zoneKey": "US",
"lonlat": [45.5225, -122.992],
"region": "North America",
Copy link
Member

Choose a reason for hiding this comment

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

A bit unclear I'll admit but I'm pretty sure this needs to be:

Suggested change
"region": "North America",
"region": "us-hillsboro",

(this applies to all of them)

The key is made up of the provider + region so the validation functions works properly.

Otherwise I think it looks good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the region

"ovh-us-hillsboro": {
"provider": "ovh",
"displayName": "Hillsboro",
"zoneKey": "US",
Copy link
Member

Choose a reason for hiding this comment

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

Zone key should also be more specific and not use a aggregate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use the internal OVH key now to build the key :
ovh-us-hillsboro => ovh-hil

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think Victor meant the zoneKey that tells Electricity Map the electrical grid region where the data centre is located.

"ovh-bhs": {
"provider": "ovh",
"displayName": "Beauharnois",
"zoneKey": "CA",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"zoneKey": "CA",
"zoneKey": "CA-QC",

"ovh-tor": {
"provider": "ovh",
"displayName": "Toronto",
"zoneKey": "CA",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"zoneKey": "CA",
"zoneKey": "CA-ON",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you're right, I hadn't realised it was the zone key of the electricity map, so I've changed it for the countries concerned

"ovh-us-hillsboro": {
"provider": "ovh",
"displayName": "Hillsboro",
"zoneKey": "US",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think Victor meant the zoneKey that tells Electricity Map the electrical grid region where the data centre is located.

},
"ovh-mum": {
"provider": "ovh",
"displayName": "Mumbai ",
Copy link
Collaborator

Choose a reason for hiding this comment

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

extra space 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx updated

Copy link
Member

VIKTORVAV99 commented Jun 2, 2025

@james I'm a bit bussy with the API changes at the moment so could you maybe take a look at reviewing this one and bring it over the finish line?

@VIKTORVAV99 VIKTORVAV99 requested a review from jbdietrich June 2, 2025 17:05
@@ -898,5 +898,145 @@
"status": "operational",
"source": "https://docs.aws.amazon.com/global-infrastructure/latest/regions/aws-regions.html",
"operationalSince": null
},
"ovh-hil": {
Copy link
Contributor

Choose a reason for hiding this comment

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

The linting error you see in CI is because we expect the ID to match the provider + the region. So we'd expect the key to be ovh-us-hillsbro and so on for all of the ones you added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I revert back the code to be compatible.

Copy link
Contributor

@jbdietrich jbdietrich left a comment

Choose a reason for hiding this comment

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

Thanks!

@jbdietrich jbdietrich enabled auto-merge (squash) June 3, 2025 17:29
@jbdietrich jbdietrich merged commit cd56e32 into electricitymaps:master Jun 3, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants