-
Notifications
You must be signed in to change notification settings - Fork 1
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
Replace Maxmind with IP-API #18
Conversation
simpler and does not require api or account. this allows us to remove utiliation of maxmind client and perform geo location from analytics package directly
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Looks good to me! No need to make that additional check for throttling now, I don't think.
if(geoData.status === "success") { | ||
countryISOCode = geoData.countryCode; | ||
} else { | ||
throw new Error(geoData.message); |
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 think we probably want to specifically check for the 429 response code so we can log something else if we are throttled.
@@ -26,7 +26,7 @@ importers: | |||
version: link:packages/tsconfig | |||
turbo: | |||
specifier: latest | |||
version: 1.12.2 | |||
version: 1.10.16 |
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.
weird that this got downgraded...
Geo location provider
maxmind
has a complicated workflow to set up an account and get theirMAXMIND_ACCOUNT_ID
andMAXMIND_LICENSE_KEY
. Team decided to replace Maxmind with an alternative way to get country code from IP address that does not require users to create accounts or generate API keys.IP-API provides a free geolocation API with no API key or account setup required. This PR replaces maxmind with ip-api. This allows us to move IP geolocation within the analytics package directly and remove it from Fresco in PR #81
Before merge: