green-coding-solutions / eco-ci-energy-estimation

Eco CI Energy estimation for Github Actions Runner VMs
MIT License
48 stars 10 forks source link

use HTTPS when accessing ip-api.com #77

Closed vszakats closed 3 weeks ago

ArneTR commented 3 weeks ago

That seems like a very reasonable PR, thanks for this!

@ribalba Was there a reason for using HTTP? Did you experience any issues with their HTTPS endpoint when you wrote the code initially?

ribalba commented 3 weeks ago

It states in the documentation https://ip-api.com/docs/api:json:

SSL (HTTPS)
256-bit SSL encryption is not available for this free API. Please see our [pro service](https://members.ip-api.com/).

It also doesn't seem to work. When I call https://ip-api.com/json/ I get

{"status":"fail","message":"SSL unavailable for this endpoint, order a key at https://members.ip-api.com/"}

And all the urls in the documentation are without SSL.

vszakats commented 3 weeks ago

Oh, selling HTTPS as an extra. That's a bit unexpected nowadays, but it's also not the first service doing it.

Though there are plenty of alternatives, I don't know any one of them enough to suggest a good one. Yet, it'd be nice to close the loop and do HTTPS everywhere. Esp. for folks using this action on private runners.

ribalba commented 3 weeks ago

In CarbonDB we also use:

which both work great. I will change to code to a service that uses https. I chose ip-api because it was the service with the most free requests. But this doesn't really matter here as we will get a new external ip each time we get a new VM

ribalba commented 3 weeks ago

I create a new PR here https://github.com/green-coding-solutions/eco-ci-energy-estimation/pull/78

This changes the ip resolver service to something that gives us https for free.

Thx @vszakats for the feedback and PR.

vszakats commented 3 weeks ago

Thank you @ribalba and @ArneTR for your responses and the PR.