Skip to content
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

Fix: Switch to restcountries v3 API and use common country name #158

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

sameh-farouk
Copy link
Member

@sameh-farouk sameh-farouk commented Feb 20, 2024

What's changed:

  • Switching from using official counties' names to common names introduced on the latest version of restcountries API.

Related Issues:

Description:

This is a temporary, easy workaround fix for the issue #148.

However, although it should resolve the mismatch for the reported countries, it is important to note that this fix may not work for all countries as the process of joining data from different APIs based on country names is not reliable.

To elaborate, in our squid processor, we fetch cities from the repository available at https://raw.githubusercontent.com/shivammathur/countrycity/master/data/geo.json and countries from the https://restcountries.com/ API. We then join these data based on the country name. Similarly, in the Gridproxy, we fetch node data from the chain and county data from GraphQL and join it based on the country name.

This process is not always accurate as there are often alternative spellings for country names. Hence, unless you are using the same data source, it's not guaranteed to find a match.

To properly resolve this issue, I suggest using the same API or a more unique identifier such as country code. However, this would require changes in different services.

@sameh-farouk sameh-farouk merged commit fb3b187 into master Feb 22, 2024
1 check passed
@sameh-farouk sameh-farouk deleted the master-fix-use-common-country-name-issu-148 branch February 22, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants