Skip to content

Always initialize geocoder if geolocation data file exists#10319

Merged
aduth merged 1 commit intomainfrom
aduth-geocoder-load-local
Mar 27, 2024
Merged

Always initialize geocoder if geolocation data file exists#10319
aduth merged 1 commit intomainfrom
aduth-geocoder-load-local

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Mar 26, 2024

🛠 Summary of changes

Updates geocoder initializer to always load the geocoder file if it exists, regardless of environment.

This is to avoid future confusion like I experienced when following our instructions to set up geolocation and it not working as instructed.

📜 Testing Plan

  1. Follow the instructions to set up geolocation
    1. Alternatively, download live database from S3
  2. Verify geocoder produces real data
    1. rails c
    2. Geocoder.search(Faker::Internet.ip_v4_address).first
    3. Verify output is something other than the fake default

changelog: Internal, Geocoder, Initialize geocoder in local development if data file present
@aduth aduth changed the title Use geolocation data if it exists Always initialize geocoder if geolocation data file exists Mar 26, 2024
@aduth
Copy link
Contributor Author

aduth commented Mar 26, 2024

We could keep the production check for the eager initialization here, if it helps avoid long start times in local development 🤷

Geocoder.search('1.2.3.4') # the datasource is lazily loaded, make sure it eager loads

GEO_DATA_FILEPATH = Rails.root.join(IdentityConfig.store.geo_data_file_path).freeze

if Rails.env.production? && File.exist?(GEO_DATA_FILEPATH)
if File.exist?(GEO_DATA_FILEPATH)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is probably better served by a feature flag if we do want it to be configurable.

@aduth aduth merged commit ba49f97 into main Mar 27, 2024
@aduth aduth deleted the aduth-geocoder-load-local branch March 27, 2024 12:19
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.

2 participants