search

DigitalOceanPHP / Client

DigitalOcean API v2 client for PHP
MIT License
710 stars 205 forks source link

[4.1] Change floating IP class to handle IP strings instead of forced int values #270

Closed LewisSmallwood closed 3 years ago

LewisSmallwood commented 3 years ago

Fixes #269

GrahamCampbell commented 3 years ago

Does the API really not allow someone to pass an integer ID (or did DigitalOcean just document that you can't, even though it does actually work)? If this is the case, we should accept string|int. Otherwise, this PR looks good.

GrahamCampbell commented 3 years ago

Try and fix StyleCI for floating IP changes

You can download the raw diff from StyleCI and apply it to get the exact fixes. Alternatively, you can leave the build broken - that is also fine. StyleCI applies its fixes after the PR is merged, so contributors don't have to worry about code style. See https://github.com/DigitalOceanPHP/Client/blob/4.1/.github/CONTRIBUTING.md. :)

mrsimonbennett commented 3 years ago

Does the API really not allow someone to pass an integer ID (or did DigitalOcean just document that you can't, even though it does actually work)? If this is the case, we should accept string|int. Otherwise, this PR looks good.

I just checked this @GrahamCampbell as I was a bit surprised https://developers.digitalocean.com/documentation/v2/#floating-ips

It seems they use the IP as the identication and do not return an INT based id for them. I am unsure if this has changed.

mrsimonbennett commented 3 years ago

Actually looking into my older library it looks like it was a simple mistake when adding typehinting

https://github.com/DigitalOceanPHP/Client/commit/96d1f23b739ffd8e08de103c9c9686b4142b18c1

LewisSmallwood commented 3 years ago

Does the API really not allow someone to pass an integer ID (or did DigitalOcean just document that you can't, even though it does actually work)? If this is the case, we should accept string|int. Otherwise, this PR looks good.

I can confirm that it is strings only, there doesn't actually appear to be an integer resource ID for floating IPs. Although, if you take an action like assign and look at the action entity it returns, there is a resource ID attached which I assume is the floating IP resource ID. But passing this to any of the methods doesn't work.

GrahamCampbell commented 3 years ago

Excellent. Thank you, everyone. 🍻