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 magicKey breaking ESRI geocoding #68

Merged
merged 5 commits into from
Jan 22, 2024
Merged

Fix magicKey breaking ESRI geocoding #68

merged 5 commits into from
Jan 22, 2024

Conversation

rajadain
Copy link
Collaborator

Overview

At some point the ESRI /findAddressCandidates endpoint stopped returning the Loc_name attribute if magicKey is specified. Since the endpoint is not open source and they don't seem to have a changelog that I could find, I don't know exactly when this happened, but reports in MMW (WikiWatershed/model-my-watershed#3604, WikiWatershed/model-my-watershed#3605) seem to indicate it has been the case for a while.

This fixes that by making that attribute optional.

Also adds a test for this. Also adds warning logging for when other keys are missing.

Demo

You can see MMW production not working using the main library, and my local version that uses this branch's omgeo working correctly:

2024-01-19.18.42.20.mp4

Closes #67

This is currently failing, to replicate the bug reported in #67.
Will add a fix for this next.
Previously we assumed that Loc_name would always be provided, given
that we ask for it in the requested field list. However, when using
the magicKey, the Loc_name is not provided, causing geocoding to
fail. By making it optional, we allow those geocoding requests to
succeed.

Also, add a WARNING to the log when there's a missing key, instead
of just silently passing, for better observability.
This new value is used for many responses now, need to include it
for the tests to pass.
@rajadain
Copy link
Collaborator Author

Tests are failing because the API keys setup in this repository may be out of date?

Disable Google test if credentials are missing.

Not sure what was wrong with the Philadelphia address for Census
purposes, but switching to an Alexandria address fixes that.
Copy link

@rachelekm rachelekm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Success! Updated my local python-omgeo package to your branch and location search works again as expected.

@rajadain
Copy link
Collaborator Author

Thanks for reviewing! Disabled some expired token dependent tests in 99f6963, and created #69 to refresh the tokens and re-enable the tests. Going to merge this now, and will create a new release.

@rajadain rajadain merged commit 82d7363 into azavea:develop Jan 22, 2024
1 check passed
@rajadain rajadain deleted the tt/67/fix-magickey-geocoding branch January 22, 2024 21:48
rajadain added a commit to WikiWatershed/model-my-watershed that referenced this pull request Jan 22, 2024
Location search was broken because of azavea/python-omgeo#67.
By upgrading to the latest version of omgeo that includes azavea/python-omgeo#68,
this issue is fixed.

PyPI release of omgeo is blocked for now azavea/python-omgeo#70,
so we use the tag directly from GitHub instead.
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.

magicKey specification breaks geocoding
2 participants