Skip to content

USPS Location Caching & Fallback Proof of Concept#7790

Closed
allthesignals wants to merge 10 commits intomainfrom
wmg/8508-usps-fallback-spike
Closed

USPS Location Caching & Fallback Proof of Concept#7790
allthesignals wants to merge 10 commits intomainfrom
wmg/8508-usps-fallback-spike

Conversation

@allthesignals
Copy link
Contributor

LG-8508

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should set up a batch geocoding interface for this instead, to save on network requests.

Copy link
Contributor Author

@allthesignals allthesignals Feb 7, 2023

Choose a reason for hiding this comment

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

We should have it lookup locations by unique combo of address/city/state/zip before geocoding those addresses.... "Upsert" behavior in order to track updates to location hours etc.

Copy link
Contributor Author

@allthesignals allthesignals Feb 7, 2023

Choose a reason for hiding this comment

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

Might be very tricky detecting removed locations. In a given cluster, it could convert to polygons and query for intersecting points, then drop missing points. 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is wrong... need to use st_within and st_buffer

Copy link
Contributor

Choose a reason for hiding this comment

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

would ST_DWithin work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mitchellhenke yeah, I think so, but it's not included on arel_table as a convenience method. I think writing postgis queries directly is probably the easiest way, but would like to see if i can't leverage those convenience methods:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

interesting, I would have hoped it is!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think most people probably just do it directly as SQL queries. Not sure what else I get from using these named methods.

@allthesignals allthesignals force-pushed the wmg/8508-usps-fallback-spike branch 2 times, most recently from 3aea376 to fc2862a Compare February 7, 2023 21:53
@allthesignals allthesignals force-pushed the wmg/8508-usps-fallback-spike branch from fc2862a to a9a7826 Compare February 7, 2023 21:58
include RenderConditionConcern
include UspsInPersonProofing
include EffectiveUser
include UspsInPersonProofing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is listed twice.....

@allthesignals allthesignals force-pushed the wmg/8508-usps-fallback-spike branch from f8f8ab3 to 9599826 Compare February 8, 2023 00:52
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this is neat and works pretty well... will update when dupe is found based on these constraints.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unclear if this makes the uniqueness constraint I added to the migration unnecessary...

Copy link
Contributor

Choose a reason for hiding this comment

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

a post office doesn't have an identifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sadly it does not

@allthesignals allthesignals force-pushed the wmg/8508-usps-fallback-spike branch from 1553daa to 069f051 Compare February 8, 2023 02:57
@allthesignals allthesignals force-pushed the wmg/8508-usps-fallback-spike branch from 069f051 to 454e636 Compare February 8, 2023 02:58
@allthesignals allthesignals force-pushed the wmg/8508-usps-fallback-spike branch 2 times, most recently from 0e158bd to a3300ca Compare February 8, 2023 06:40
@allthesignals allthesignals force-pushed the wmg/8508-usps-fallback-spike branch from a3300ca to e90a619 Compare February 8, 2023 06:55
Comment on lines +12 to +13
lonlat::geography,
'SRID=#{WGS84_SRID};POINT(#{longitude} #{latitude})'::geography,
Copy link
Contributor

@mitchellhenke mitchellhenke Feb 8, 2023

Choose a reason for hiding this comment

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

We may want to consider an index along the lines of

CREATE INDEX idx_usps_ipp_cached_locations_lonlat_gist ON
  usps_ipp_cached_locations USING GIST (geography(lonlat));

@zachmargolis
Copy link
Contributor

Is this prototype still in progress? If not, can we close it out to have fewer open PRs? (we can always re-open later if/when we choose to pick it back up)

zachmargolis added a commit that referenced this pull request Apr 11, 2024
- Code that leveraged this never landed on main (#7790)

* Comment out postgis extension migration because postgis was removed from CI image

changelog: Internal, Source code, Remove PostGIS references
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