Skip to content
This repository has been archived by the owner on Jun 30, 2023. It is now read-only.

Integrate geocode into build #599

Draft
wants to merge 38 commits into
base: 563-Add-Devcontainer
Choose a base branch
from

Conversation

SashaWeinstein
Copy link
Contributor

Medium PR, could use a second reviewer if Max is interested 🌍

Addresses issues #597 and #598

Existing implementation

Currently the geocoding is done by the data sync action that sends the geocoded data to data library.
The downside of this implementation is that if the data sync hasn't been done recently, the data can be out of date. It's also hard to test changes.

New implementation

Geocode is run on our local machines from a devcontainer. I think this is better as having a linear pipeline that can be built in from one command is best practice

Changes to HNY

Inner join makes the hny_devdb table more readable

@SashaWeinstein SashaWeinstein requested a review from a team October 18, 2022 16:40
@SashaWeinstein
Copy link
Contributor Author

Also I save HNY_lookup to a table instead of keeping it as a common table expression as it makes the code easier to debug

Copy link
Contributor

@td928 td928 left a comment

Choose a reason for hiding this comment

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

pretty cool! run it and worked on my machine. I am thinking whether one potential next step here is to run this branch on the github actions 1. to see if it works 2. create the qaqc file so we can plug into the app to see if the spatial stuff is roughly in line with the last few versions

went to look at the github actions run with this PR. I guess maybe the workflow needs some adjustment first?

@td928
Copy link
Contributor

td928 commented Oct 18, 2022

also flagging that the mc installation is needed for devcontainer

bash/config.sh: line 113: mc: command not found

@SashaWeinstein
Copy link
Contributor Author

Ok word, so next steps are mini io installation and then run on github actions and review output on app?

@td928
Copy link
Contributor

td928 commented Oct 18, 2022

Ok word, so next steps are mini io installation and then run on github actions and review output on app?

see my comments above the github actions seem not to be working for geocoding for some other reason as well. Didn't dig too deep to the cause

@SashaWeinstein SashaWeinstein marked this pull request as draft October 21, 2022 14:15
@SashaWeinstein
Copy link
Contributor Author

This is a work in progress as I haven't gotten the docker compose run to work yet. I'm not stuck and it doesn't seem all that hard, I just hard to move to other things.
The goal here is for devDB to run like facDB. The next step is to get the docker compose stack to work.
I hope that this work gets picked up eventually. I think the enhancement is close to being done and will make future builds faster.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants