-
Notifications
You must be signed in to change notification settings - Fork 76
Enhanced Interfaces: Add support for Linode-related endpoints and fields #533
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: proj/enhanced-interfaces
Are you sure you want to change the base?
Enhanced Interfaces: Add support for Linode-related endpoints and fields #533
Conversation
9d231d3
to
0d67823
Compare
def ipv6_range_allocate( | ||
self, | ||
prefix_length: int, | ||
route_target: Optional[str] = None, | ||
linode: Optional[Union[Instance, int]] = None, | ||
**kwargs, | ||
) -> IPv6Range: |
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.
This endpoint wasn't added as part of Enhanced Interfaces but it was oddly missing from the SDK. I added it here to unblock some of the IPv6 interface test cases below.
Let me know if this is worth splitting out into a separate PR 🙂
# NOTE: We do not implement this as a Property because Property does | ||
# not currently have a mechanism for 1:1 sub-entities. |
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.
Any thoughts on whether this would be worth implementing as a separate body of work?
1b68419
to
ed25e3b
Compare
ed25e3b
to
5739a51
Compare
""" | ||
|
||
params = { | ||
"firewall_id": firewall, |
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.
The API complains about the firewall_id
key if it's fully excluded from the request, but doesn't complain when the key if explicitly specified as null:
[400] firewall_id: No default firewall has been defined for this interface type. Please define a default firewall or provide an ID (or 'null') for a specific firewall.
Does anyone know if this was an intentional API decision or if it's just a bug?
vpc=LinodeInterfaceVPCOptions( | ||
subnet_id=subnet.id, | ||
ipv4=LinodeInterfaceVPCIPv4Options( | ||
# TODO (Enhanced Interfaces): Not currently working as expected |
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.
I was running into something that seemed like an API bug here; will report when I have some time
"vpc": Property(mutable=True, json_object=LinodeInterfaceVPC), | ||
} | ||
|
||
def get_firewalls(self, *filters) -> List[Firewall]: |
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.
I used the name get_firewalls(...)
rather than firewalls(...)
here to prevent ambiguity between this and similar property methods.
Any thoughts on this? I'd be happy to revert it if we feel it's too out of line with the rest of the SDK 🙂
edit: Reverted to align with Instance(...)
non-property firewalls(...)
method.
37c3a31
to
bbb021e
Compare
interfaces = kwargs.get("interfaces", None) | ||
if interfaces is not None and isinstance(interfaces, Iterable): | ||
kwargs["interfaces"] = [ | ||
i._serialize() if isinstance(i, ConfigInterface) else i | ||
for i in interfaces | ||
] |
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.
No longer necessary due to _flatten_request_body_recursive(drop_null_keys(params))
below
📝 Description
This pull request adds support for the following API interfaces and fields introduced as part of the VPC Linodes Enhanced Interfaces project:
This includes the following additions:
LinodeInterface
and associated JSONObjectsLinodeInterfacesSettings
and associated JSONObjectsInstance().interface_create(...)
Instance().interfaces_settings
(property method)Instance().interfaces
(property method)NetworkingGroup().ipv6_range_allocate(...)
- Unrelated; dependencyLinodeGroup().instance_create(...)
-interfaces
,interface_generation
,network_helper
Integration Test Suite Run: https://github.com/linode/linode_api4-python/actions/runs/14862125137/job/41729485455
✔️ How to Test
The following test steps assume you have pulled down this PR locally and run
make install
.Unit Testing
Integration Testing
Manual Testing