fix: update geolocation fallback API and use static country code mapping#1104
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1104 +/- ##
==========================================
+ Coverage 78.21% 78.25% +0.04%
==========================================
Files 38 38
Lines 3631 3638 +7
==========================================
+ Hits 2840 2847 +7
Misses 791 791 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks Lionel for testing your previous PR in a production environment. Can you run I will prefer to avoid adding a dependency. But it is not really related to your PR. I think we could handle this later. Maybe @SaboniAmine could add the 2 letters code in the carbon intensity dataset from https://github.com/pycountry/pycountry/blob/main/src/pycountry/databases/iso3166-1.json in his work on https://github.com/mlco2/codecarbon-hardware-data . So we will remove the dependency later. |
7d4ac4b to
89e5a62
Compare
|
@benoit-cty ran the pre-commit script and fixed the code formatting issue. I understand we want to keep dependency to the minimum, though highlighting we really traded a weak network dependency for a direct code dependency, which should improves the library resiliency. |
benoit-cty
left a comment
There was a problem hiding this comment.
Thanks, I will open an issue to remove this dependency later.
|
Hi, thanks for this contribution! |
Issue created : https://github.com/mlco2/codecarbon-hardware-data/issues/3 |
Description
Fixes geolocation fallback API call
Issue 1
The fallback API will always fail when using the HTTPS endpoint
Using the HTTP endpoint is not great because this could expose the application location to eavesdropper and be vulnerable to man-in-the-middle.
Issue 2
If the fallback API call fails, it reports on the wrong URL on the exception: https://github.com/ucodia/codecarbon/blob/ebc8d1a3f5dcae9013dcbe6883d05e711ad6ce28/codecarbon/external/geography.py#L136
Non-issue 1
The current fallback relies on another API to map the 2 letters country code to a 3 letters code. This is static data that almost never changes, by switching to a static mapping provider we can make the fallback more resilient.
Related Issue
Fixes #1101
Motivation and Context
Certain IP ranges are not covered by the primary API and therefore a fallback is necessary otherwise CodeCarbon client will estimate on the wrong energy mix.
How Has This Been Tested?
Screenshots (if appropriate):
N/A
Types of changes
What types of changes does your code introduce? Put an
xin all the boxes that apply:Checklist:
Go over all the following points, and put an
xin all the boxes that apply.