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.

Sign up for GitHub

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

Jump to bottom

Use Mapbox geocoder + GitHub Actions Automation #3

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Use Mapbox geocoder + GitHub Actions Automation #3

wants to merge 18 commits into from

Conversation

riledigital
Copy link

@riledigital riledigital commented May 14, 2022 •
edited
Loading

This is an amazing project and I have looked for months for a map that provides this information!

I added a bunch of changes that will hopefully ease maintenance, facilitate regular data updates (daily), and possibly increase performance just a smidge.

(Sorry for all the changes, I only meant to swap out the geocoder and got a little carried away with some OOP fun)

Geocoding

  • Refactor/organize code related to geocoding into a GeocodingManager facade class
  • Switch out Google Geocoder API for Mapbox API (current usage of 375/run (375 * 30 = 11K) is far under Mapbox's max of 300K requests/month); I couldn't test this project without potentially triggering Google Cloud billing for the Geolocation API... 😇
  • Include Mapbox GL JS as a dep through Parcel
  • Remove d3 as a dependency
  • Provide GeoJSON for user to download (may be of interest for public health/research etc; easier to load into GIS clients)

Automation

  • Write a cached Actions workflow that scrapes the data in an Actions job runner, provides the GeoJSON in a build artifact and deploys to Netlify

Architecture changes

  • Remove dependency on wait times dataset; map points currently don't load if wait times file doesn't load
  • Decouple map client from data files- should reduce bundle size and speed of initial load
  • Use Mapbox source+style+layer pattern
  • modularize wait times code and remove data loading dependency on wait times
  • modularize popup code
  • Fix node to v16 via nvm
  • convert server-side CJS to ES6 modules

@netlify
Copy link

netlify bot commented May 14, 2022 •
edited
Loading

👷 Deploy request for nyc-run-covid-testing pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 5ad3c20

riledigital added 12 commits May 15, 2022 10:41
@riledigital
# This is a combination of 6 commits.
b1ff70b 
# This is the 1st commit message:

GH Actions for scrape + deploy

Test gh action

Attempt to trigger through push

Update deps

Clear Parcel autoprefixer deprecation

# This is the commit message efrymire#2:

Remove d3 dep

# This is the commit message efrymire#3:

Add in generic geocoder

Add scrape+geocode result to GH Action Artifacts

Pass secret to GH Action

Add specific error logging

Downgrade node

Remove null format

Make sure secrets actually exist

# This is the commit message #4:

Add ci-deploy command

Add engine hint

# This is the commit message #5:

Add GeoJSON and static folder

# This is the commit message #6:

Add gitignore and expand mkdirs

Remove mistake

Change null handling
@riledigital
GH Actions for scrape + deploy
2905e3a 
Test gh action

Attempt to trigger through push

Update deps

Clear Parcel autoprefixer deprecation

Remove d3 dep

Add in generic geocoder

Add scrape+geocode result to GH Action Artifacts

Pass secret to GH Action

Add specific error logging

Downgrade node

Remove null format

Make sure secrets actually exist

Add ci-deploy command

Add engine hint

Add GeoJSON and static folder

Add gitignore and expand mkdirs

Remove mistake

Change null handling

GH Actions for scrape + deploy

Test gh action

Attempt to trigger through push

Update deps

Clear Parcel autoprefixer deprecation

Remove d3 dep

Add in generic geocoder

Add scrape+geocode result to GH Action Artifacts

Pass secret to GH Action

Add specific error logging

Downgrade node

Remove null format

Make sure secrets actually exist

Add ci-deploy command

Add engine hint
@riledigital
Add GeoJSON and static folder
58b35b3
@riledigital
Add gitignore and expand mkdirs
df1c85e 
Remove mistake

Change null handling
@riledigital
README + Netlify Action
73852a7 
Swap out Netlify action

Add caching to action

Correct caching

Remove extra key from netlify

Schedule every 1AM
@riledigital
Add Mapbox through npm
bf99153
@riledigital
System font + Facade
284815e
@riledigital
Modularize+refactor MapManager to maximize async load
4f27f26
@riledigital
Remove indenting for stringify and refactor
cbc59d1
@riledigital
Remove output data indenting
7c8afda
@riledigital
Remove text from markers and add downlaod
ed5c346 
Refactor
@riledigital
Fix import for Popup
b8462d9
@efrymire
Copy link
Owner

efrymire commented May 15, 2022 •
edited
Loading

awesome thanks @riledigital ! I can take a look later tonight -- is it ready for my review?

I see I have to approve your deploy previews... LMK when you'd like to test the deploy and we can check the preview.

@riledigital
Copy link
Author

riledigital commented May 15, 2022 •
edited
Loading

Ready to test the deploy + review!

The GH Action run is successful on my end with my Netlify tokens plugged in—you'll need to set up the tokens in the repo (i just documented this in the readme)

Screen Shot 2022-05-15 at 11 22 06 AM

Also again sorry for the massive PR! No rush to review this at all; Just wanted to do this for fun and practice; figured I could submit a PR if it's helpful to anyone!

@riledigital riledigital marked this pull request as ready for review May 15, 2022 15:24
@riledigital
Copy link
Author

riledigital commented May 15, 2022 •
edited
Loading

Ah I think I know what is going on here... Parcel tries to bundle references and is trying to bundle the centers.geojson data file (something I'm trying to avoid in this PR). It looks like the GitHub Netlify integration is pulling the repo and running the build command (without running the scraper first) so it fails.

I'm going to try to stop parcel from resolving that file... Fixed!

I could also fix this by including a .netlify.toml that specifies npm run ci-deploy as the build command, but that means the scraper will: (also done!)

  • require you to add your Mapbox API key to the Netlify Environment vars
  • consume your Netlify build minutes (approx ~1 minute a day, 30 min a month— netlify free tier is currently 300 mins a month— if the runner is comparable to the GH one). I'm cautious about eating up ppl's metered build minutes + being a little stingy so i would recommend using GH Actions free compute since it's already set up 😅
  • you will also not be able to download the data through GH Actions build artifacts (but i guess this doesn't really matter that much since the deployed site includes the data file anyway).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@riledigital @efrymire