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

New database #9

Merged
merged 10 commits into from
Dec 9, 2024
Merged

New database #9

merged 10 commits into from
Dec 9, 2024

Conversation

t-downing
Copy link
Collaborator

@t-downing t-downing commented Dec 2, 2024

Key changes:

  • .github/workflows/run_update_exposure.yml: add step to trigger second workflow
  • .github/workflows/run_update_raster_stats.yml: created to calculate raster stats, and configured to be portable to another repository
  • pipelines/update_database.py: deleted as we will no longer use this database table
  • pipelines/update_raster_stats.py: script created to calculate raster stats and store in new database table
  • src/utils/database.py: define schema of new database table

Also:

  • adding Mali

So, pipeline now would work by:

  • calculating exposure raster with run_update_exposure.yml
  • calculating exposure raster stats with run_update_raster_stats.yml
    • this workflow is configured to be triggered by the first workflow, and can eventually be moved to another repository

Note that I left in the .parquet calculation in floodscan.calculate_recent_flood_exposure_rasterstats() so we still have these as a backup that gets updated daily. When we are more sure about the new database, and are writing to it daily, we can get rid of this function.

@t-downing t-downing marked this pull request as ready for review December 4, 2024 19:16
@t-downing t-downing requested a review from hannahker December 4, 2024 19:32
Base automatically changed from database-outputs to exposure-pipeline December 4, 2024 21:09
src/constants.py Outdated Show resolved Hide resolved
DEV_BLOB_SAS: ${{ secrets.DEV_BLOB_SAS }}
AZURE_DB_PW_DEV: ${{ secrets.AZURE_DB_PW_DEV }}
AZURE_DB_PW_PROD: ${{ secrets.AZURE_DB_PW_PROD }}
AZURE_DB_UID: ${{ secrets.AZURE_DB_UID_DEV }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe want to change the name of this in secrets to not reference DEV -- not essential though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah in general I think having a better naming structure for our env vars would be good. Following up with Mike on including them as organization secrets as well, which would force us to be consistent.

Copy link
Collaborator

@hannahker hannahker left a comment

Choose a reason for hiding this comment

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

Awesome! Only minor comments/clarifications. This looks good to me.

Not to be done in this PR, but I'm assuming next steps from here would be:

  • Reconfigure processing of exposure rasters to contain full geographic scope (ie. not split by iso3)
  • Then replace the raster stats computations here with what we have in ds-raster-stats repo.

@t-downing
Copy link
Collaborator Author

Not to be done in this PR, but I'm assuming next steps from here would be:

Reconfigure processing of exposure rasters to contain full geographic scope (ie. not split by iso3)
Then replace the raster stats computations here with what we have in ds-raster-stats repo.

Yup! Noted as #12, #13, OCHA-DAP/ds-raster-stats#31

@t-downing t-downing merged commit 4b66fcb into exposure-pipeline Dec 9, 2024
@t-downing t-downing deleted the new-database branch December 9, 2024 20:06
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