Skip to content
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

Jump to bottom

Add support for Metros #110

Merged
merged 5 commits into from
May 20, 2021
Merged

Add support for Metros #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3e608dc
add support for Metros
displague Apr 12, 2021
35da575
add get_metros fixtures
displague Apr 19, 2021
8b5722c
handle null facility and metro in ip/vlan responses
displague Apr 22, 2021
33bc6b1
remove redundant None in data.get(foo, None)
displague May 18, 2021
275314f
Add support empty Facility for IPAddress
tyzhnenko May 18, 2021
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ manager = packet.Manager(auth_token="yourapiauthtoken")

device = manager.create_device(project_id='project-id',
hostname='node-name-of-your-choice',
plan='baremetal_1', facility='ewr1',
plan='baremetal_1', metro='sv',
operating_system='ubuntu_18_04')
print(device)
```
Expand Down
1 change: 1 addition & 0 deletions packet/Device.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def __init__(self, data, manager):
self.customdata = data.get("customdata", None)
self.operating_system = OperatingSystem(data["operating_system"])
self.facility = data.get("facility")
self.metro = data.get("metro")
self.project = data.get("project")
self.ssh_keys = data.get("ssh_keys")
self.project_lite = data.get("project_lite")
Expand Down
1 change: 1 addition & 0 deletions packet/DeviceBatch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def __init__(self, data):
self.plan = data.get("plan")
self.operating_system = data.get("operating_system")
self.facility = data.get("facility")
self.metro = data.get("metro")
self.quantity = data.get("quantity")

def __str__(self):
Expand Down
7 changes: 6 additions & 1 deletion packet/IPAddress.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# SPDX-License-Identifier: LGPL-3.0-only

from .Facility import Facility
from .Metro import Metro


class IPAddress:
Expand All @@ -22,14 +23,18 @@ def __init__(self, data, manager):
self.customdata = data.get("customdata")
self.project = data.get("project")
self.project_lite = data.get("project_lite")
self.facility = Facility(data.get("facility"))
self.details = data.get("details")
self.assigned_to = data.get("assigned_to")
self.interface = data.get("interface")
self.network = data.get("network")
self.address = data.get("address")
self.gateway = data.get("gateway")

facility = data.get("facility")
self.facility = Facility(facility) if facility else None
metro = data.get("metro")
self.metro = Metro(metro) if metro else None

def delete(self):
return self.manager.call_api("ips/%s" % self.id, type="DELETE")

Expand Down
59 changes: 47 additions & 12 deletions packet/Manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from .SSHKey import SSHKey
from .Project import Project
from .Facility import Facility
from .Metro import Metro
from .OperatingSystem import OperatingSystem
from .Volume import Volume
from .BGPConfig import BGPConfig
Expand Down Expand Up @@ -41,6 +42,14 @@ def list_facilities(self, params={}):
facilities.append(facility)
return facilities

def list_metros(self, params={}):
data = self.call_api("metros", params=params)
metros = list()
for jsoned in data["metros"]:
metro = Metro(jsoned)
metros.append(metro)
return metros

def list_plans(self, params={}):
data = self.call_api("plans", params=params)
plans = list()
Expand Down Expand Up @@ -121,8 +130,8 @@ def create_device(
project_id,
hostname,
plan,
facility,
operating_system,
facility="",
operating_system="",
always_pxe=False,
billing_cycle="hourly",
features={},
Expand All @@ -139,11 +148,11 @@ def create_device(
hardware_reservation_id="",
storage={},
customdata={},
metro="",
):

params = {
"billing_cycle": billing_cycle,
"facility": facility,
"features": features,
"hostname": hostname,
"locked": locked,
Expand All @@ -157,6 +166,10 @@ def create_device(
"userdata": userdata,
}

if metro != "":
params["metro"] = metro
if facility != "":
params["facility"] = facility
Comment on lines +169 to +172
Copy link
Member

Choose a reason for hiding this comment

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

would it be ok to have both metro and facility? What if they are incompatible? IIRC for these types of things we let the API handle the error checking/response right?

Copy link
Member

Choose a reason for hiding this comment

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

Also, there's no check to ensure at least one of facility or metro is specified. This can be seen as a bit of change in behavior where previously facility was required and check by Python.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to send {"metro": "", "facility": "ewr1"}, for example. It results in " is not a valid metro"

Copy link
Member Author

@displague displague May 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

So we avoid sending an empty string for metro or facility, by not including it in the JSON payload.

We're deliberately working around the previously required facility field.

Perhaps facility = None and metro = None would be better defaults? We don't get the same API error with {"metro": null} as payload.

Copy link
Member Author

@displague displague May 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sending null for this field should probably mean something else, and we would be getting lucky that the API treats this as a field omission (in this case). It's not documented.

I think we should try to avoid sending any value if we don't want a value (not include metro or facility in the payload in the presence of the other).

Copy link
Member

Choose a reason for hiding this comment

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

I don't want to send {"metro": "", "facility": "ewr1"}, for example. It results in " is not a valid metro"

What about:

if not metro and not facility:
    # raise an error, one of metro or facility must be specified.

We're deliberately working around the previously required facility field.

Why? It seems to me that one of metro or facility must be required, otherwise the request is garbage right?

I guess similarly, we can choose to require one-and-only-one of metro or facility to be provided (though I'm on the fence on this tbh, the user could call metro = NY, facility = NY5 and thats fine (if those are actually valid values), so I'm ok with letting the API do the validity check here))

Copy link
Member Author

@displague displague May 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The API does not allow for metro + facility in the request, even if the facility is currently included in that metro.

It's not enough to send empty strings for the opposite value. And sending null for the opposite value relies on undefined behavior.

The request that we send must either contain a non-empty facility or a non-empty metro or else the request is not valid according to the API.

One of those fields (facility) was previously required, so now it has been made optional with a default value.

This client needs to accept both fields in the parameter list and omit one of those fields from the request. The user can't toggle that behavior with the param list alone, so the function needs logic to do it.

In Packngo we got this for free at serialization time via:

type CreateDeviceInput struct {
 Facilities []string `json:"facilities,omitempty"`
 Metro string `json:"metro,omitempty"`
}

The conditional params setting in create_device is mimicking that behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm ok with letting the API do the validity check here

It results in " is not a valid metro" -- this is an API error (I didn't make that clear before).

The API is handling the validation in all cases, python is not going to throw an early error.

  • If you try to send both facility and metro to create_device, we'll pass it along to the API. And you'll get an error.
  • If you send neither to create_device, the API will give you the error.
  • If you send "" to either, the API will give you the error.

if hardware_reservation_id != "":
params["hardware_reservation_id"] = hardware_reservation_id
if storage:
Expand Down Expand Up @@ -319,6 +332,23 @@ def validate_capacity(self, servers):
else:
raise e

# servers is a list of tuples of metro, plan, and quantity.
def validate_metro_capacity(self, servers):
params = {"servers": []}
for server in servers:
params["servers"].append(
{"facility": server[0], "plan": server[1], "quantity": server[2]}
)

try:
data = self.call_api("/capacity/metros", "POST", params)
return all(s["available"] for s in data["servers"])
except PacketError as e: # pragma: no cover
if e.args[0] == "Error 503: Service Unavailable":
return False
else:
raise e

def get_spot_market_prices(self, params={}):
data = self.call_api("/market/spot/prices", params=params)
return data["spot_market_prices"]
Expand Down Expand Up @@ -405,20 +435,24 @@ def reserve_ip_address(
project_id,
type,
quantity,
facility,
facility="",
details=None,
comments=None,
tags=list(),
metro="",
):
request = {
"type": type,
"quantity": quantity,
"facility": facility,
"details": details,
"comments": comments,
"tags": tags,
}

if facility != "":
request["facility"] = facility
if metro != "":
request["metro"] = metro
Comment on lines +452 to +455
Copy link
Member

Choose a reason for hiding this comment

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

ditto

data = self.call_api(
"/projects/%s/ips" % project_id, params=request, type="POST"
)
Expand Down Expand Up @@ -575,15 +609,18 @@ def list_vlans(self, project_id, params=None):
return vlans

def create_vlan(
self, project_id, facility, vxlan=None, vlan=None, description=None
self, project_id, facility="", vxlan=None, vlan=None, description=None, metro=""
):
params = {
"project_id": project_id,
"facility": facility,
"vxlan": vxlan,
"vlan": vlan,
"description": description,
}
if facility != "":
params["facility"] = facility
if metro != "":
params["metro"] = metro
Comment on lines +620 to +623
Copy link
Member

Choose a reason for hiding this comment

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

ditto

data = self.call_api(
"projects/%s/virtual-networks" % project_id, type="POST", params=params
)
Expand Down Expand Up @@ -638,14 +675,12 @@ def create_packet_connections(self, params):
"project_id": params["project_id"],
"provider_id": params["provider_id"],
"provider_payload": params["provider_payload"],
"facility": params["facility"],
"port_speed": params["port_speed"],
"vlan": params["vlan"],
}
if "tags" in params:
body["tags"] = params["tags"]
if "description" in params:
body["description"] = params["description"]
for opt in ["tags", "description", "facility", "metro"]:
if opt in params:
body[opt] = params[opt]

data = self.call_api("/packet-connect/connections", type="POST", params=body)
return data
Expand Down
16 changes: 16 additions & 0 deletions packet/Metro.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# -*- coding: utf-8 -*-
# SPDX-License-Identifier: LGPL-3.0-only


class Metro:
def __init__(self, data):
self.id = data.get("id")
self.code = data.get("code")
self.name = data.get("name")
self.country = data.get("country")

def __str__(self):
return "%s" % self.code

def __repr__(self):
return "{}: {}".format(self.__class__.__name__, self.id)
7 changes: 6 additions & 1 deletion packet/Vlan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# SPDX-License-Identifier: LGPL-3.0-only
from packet import Project
from .Facility import Facility
from .Metro import Metro


class Vlan:
Expand All @@ -15,9 +16,13 @@ def __init__(self, data, manager):
self.vxlan = data.get("vxlan")
self.internet_gateway = data.get("internet_gateway")
self.facility_code = data.get("facility_code")
self.metro_code = data.get("metro_code")
self.created_at = data.get("created_at")
facility = data.get("facility", None)
self.facility = Facility(facility) if facility else None
metro = data.get("metro", None)
Comment on lines +21 to +23
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
facility = data.get("facility", None)
self.facility = Facility(facility) if facility else None
metro = data.get("metro", None)
facility = data.get("facility")
self.facility = Facility(facility) if facility else None
metro = data.get("metro")

self.metro = Metro(metro) if metro else None

self.facility = Facility(data.get("facility"))
try:
project_data = self.manager.call_api(
data["assigned_to"]["href"], type="GET"
Expand Down
1 change: 1 addition & 0 deletions packet/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from .Email import Email # noqa
from .Event import Event # noqa
from .Facility import Facility # noqa
from .Metro import Metro # noqa
from .OperatingSystem import OperatingSystem # noqa
from .Plan import Plan # noqa
from .Project import Project # noqa
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/get_metros.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"metros":[{"id":"0446ac38-aae0-45f5-959c-54d26c4fc3c7","name":"Phoenix","code":"px","country":"US"},{"id":"833225d2-a35a-49cf-a67c-1f84d5d11654","name":"Marseille","code":"mr","country":"FR"},{"id":"96a57b6d-c62c-41b5-ab8e-f8d63a7f9887","name":"Washington DC","code":"dc","country":"US"},{"id":"a04e45e4-4356-458c-b25a-b72cc54bdad1","name":"Atlanta","code":"at","country":"US"},{"id":"932eecda-6808-44b9-a3be-3abef49796ef","name":"New York","code":"ny","country":"US"},{"id":"108b2cfb-246b-45e3-885a-bf3e82fce1a0","name":"Amsterdam","code":"am","country":"NL"},{"id":"5afb3744-f80d-4b49-bf21-50ede9252cfd","name":"Toronto","code":"tr","country":"CA"},{"id":"d2a8e94f-2b42-4440-9c8e-8ef01defcd27","name":"Seoul","code":"sl","country":"KR"},{"id":"d50fd052-34ec-4977-a173-ad6f9266995d","name":"Hong Kong","code":"hk","country":"HK"},{"id":"f6ada324-8226-4bfc-99a8-453d47caf2dc","name":"Singapore","code":"sg","country":"SG"},{"id":"5f72cbf6-96e4-44f2-ae60-213888fa2b9f","name":"Tokyo","code":"ty","country":"JP"},{"id":"b1ac82b2-616c-4405-9424-457ef6edf9ae","name":"Frankfurt","code":"fr","country":"DE"},{"id":"d2f09853-a1aa-4b29-aa9d-682462fd8d1d","name":"Sydney","code":"sy","country":"AU"},{"id":"5f1d3910-2059-4737-8e7d-d4907cfbf27f","name":"London","code":"ld","country":"GB"},{"id":"91fa81e8-db1a-4da1-b218-7beeaab6f8bc","name":"Kansas City","code":"kc","country":"US"},{"id":"f2f0f0b3-5359-4296-a402-56abad842f56","name":"Paris","code":"pa","country":"FR"},{"id":"56c55e99-8125-4f65-af21-a8682e033804","name":"Houston","code":"ho","country":"US"},{"id":"b8ffa6c9-3da2-4cf1-b48d-049002b208fe","name":"Seattle","code":"se","country":"US"},{"id":"2991b022-b8c4-497e-8db7-5a407c3a209b","name":"Silicon Valley","code":"sv","country":"US"},{"id":"bb059cc0-0b2a-4f5b-8a55-219e6b4240da","name":"Los Angeles","code":"la","country":"US"},{"id":"60666d92-e00f-43a8-a9f8-fddf665390ca","name":"Chicago","code":"ch","country":"US"},{"id":"d3d6b29f-042d-43b7-b3ce-0bf53d5754ca","name":"Dallas","code":"da","country":"US"},{"id":"543f8059-65f6-4c2c-b7ad-6dac5eb87085","name":"Pittsburgh","code":"pi","country":"US"},{"id":"583a553a-465a-49f3-a8a9-5fa6b44aa519","name":"Detroit","code":"dt","country":"US"}]}
98 changes: 98 additions & 0 deletions test/fixtures/get_projects_438659f0_ips.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
{"ip_addresses": [{
"id": "99d5d741-3756-4ebe-a014-34ea7a2e2be1",
"address_family": 4,
"netmask": "255.255.255.254",
"created_at": "2019-07-18T08:46:38Z",
"details": null,
"tags": [],
"public": true,
"cidr": 31,
"management": true,
"manageable": true,
"enabled": true,
"global_ip": null,
"customdata": {},
"project": {},
"project_lite": {},
"facility": {
"id": "8e6470b3-b75e-47d1-bb93-45b225750975",
"name": "Amsterdam, NL",
"code": "ams1",
"features": [
"baremetal",
"storage",
"global_ipv4",
"backend_transfer",
"layer_2"
],
"address": {
"href": "#0688e909-647e-4b21-bdf2-fc056d993fc5"
},
"ip_ranges": [
"2604:1380:2000::/36",
"147.75.204.0/23",
"147.75.100.0/22",
"147.75.80.0/22",
"147.75.32.0/23"
]
},
"assigned_to": {
"href": "/devices/54ffd20d-d972-4c8d-8628-9da18e67ae17"
},
"interface": {
"href": "/ports/02ea0556-df04-4554-b339-760a0d227b44"
},
"network": "147.75.84.94",
"address": "147.75.84.95",
"gateway": "147.75.84.94",
"href": "/ips/99d5d741-3756-4ebe-a014-34ea7a2e2be1"
},
{
"id": "99d5d741-3756-4ebe-a014-34ea7a2e2be2",
"address_family": 4,
"netmask": "255.255.255.254",
"created_at": "2019-07-18T08:46:38Z",
"details": null,
"tags": [],
"public": true,
"cidr": 31,
"management": true,
"manageable": true,
"enabled": true,
"global_ip": null,
"customdata": {},
"project": {},
"project_lite": {},
"facility": {
"id": "8e6470b3-b75e-47d1-bb93-45b225750975",
"name": "Amsterdam, NL",
"code": "ams1",
"features": [
"baremetal",
"storage",
"global_ipv4",
"backend_transfer",
"layer_2"
],
"address": {
"href": "#0688e909-647e-4b21-bdf2-fc056d993fc5"
},
"ip_ranges": [
"2604:1380:2000::/36",
"147.75.204.0/23",
"147.75.100.0/22",
"147.75.80.0/22",
"147.75.32.0/23"
]
},
"assigned_to": {
"href": "/devices/54ffd20d-d972-4c8d-8628-9da18e67ae17"
},
"interface": {
"href": "/ports/02ea0556-df04-4554-b339-760a0d227b44"
},
"network": "147.75.84.94",
"address": "147.75.84.95",
"gateway": "147.75.84.94",
"href": "/ips/99d5d741-3756-4ebe-a014-34ea7a2e2be1"
}]}
Loading