-
Notifications
You must be signed in to change notification settings - Fork 4
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
Moved the feed data for the lookup tables from csv file into src/conf… #67
Changes from 3 commits
b570fd6
f96e709
f584799
21052bb
4fd0d11
8ee8223
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
""" | ||
START seed data for the lookup tables: | ||
address_state | ||
federal_regulator | ||
hmda_institution_type | ||
sbl_institution_type | ||
These are accessed in db_revisions/versions/* scripts and in test/migrations/test_lookup_tables_data_seed . | ||
""" | ||
|
||
address_state_seed = [ | ||
{"code": "AL", "name": "Alabama"}, | ||
{"code": "AK", "name": "Alaska"}, | ||
{"code": "AZ", "name": "Arizona"}, | ||
{"code": "AR", "name": "Arkansas"}, | ||
{"code": "CA", "name": "California"}, | ||
{"code": "CO", "name": "Colorado"}, | ||
{"code": "CT", "name": "Connecticut"}, | ||
{"code": "DE", "name": "Delaware"}, | ||
{"code": "FL", "name": "Florida"}, | ||
{"code": "GA", "name": "Georgia"}, | ||
{"code": "HI", "name": "Hawaii"}, | ||
{"code": "ID", "name": "Idaho"}, | ||
{"code": "IL", "name": "Illinois"}, | ||
{"code": "IN", "name": "Indiana"}, | ||
{"code": "IA", "name": "Iowa"}, | ||
{"code": "KS", "name": "Kansas"}, | ||
{"code": "KY", "name": "Kentucky"}, | ||
{"code": "LA", "name": "Louisiana"}, | ||
{"code": "ME", "name": "Maine"}, | ||
{"code": "MD", "name": "Maryland"}, | ||
{"code": "MA", "name": "Massachusetts"}, | ||
{"code": "MI", "name": "Michigan"}, | ||
{"code": "MN", "name": "Minnesota"}, | ||
{"code": "MS", "name": "Mississippi"}, | ||
{"code": "MO", "name": "Missouri"}, | ||
{"code": "MT", "name": "Montana"}, | ||
{"code": "NE", "name": "Nebraska"}, | ||
{"code": "NV", "name": "Nevada"}, | ||
{"code": "NH", "name": "New Hampshire"}, | ||
{"code": "NJ", "name": "New Jersey"}, | ||
{"code": "NM", "name": "New Mexico"}, | ||
{"code": "NY", "name": "New York"}, | ||
{"code": "NC", "name": "North Carolina"}, | ||
{"code": "ND", "name": "North Dakota"}, | ||
{"code": "OH", "name": "Ohio"}, | ||
{"code": "OK", "name": "Oklahoma"}, | ||
{"code": "OR", "name": "Oregon"}, | ||
{"code": "PA", "name": "Pennsylvania"}, | ||
{"code": "RI", "name": "Rhode Island"}, | ||
{"code": "SC", "name": "South Carolina"}, | ||
{"code": "SD", "name": "South Dakota"}, | ||
{"code": "TN", "name": "Tennessee"}, | ||
{"code": "TX", "name": "Texas"}, | ||
{"code": "UT", "name": "Utah"}, | ||
{"code": "VT", "name": "Vermont"}, | ||
{"code": "VA", "name": "Virginia"}, | ||
{"code": "WA", "name": "Washington"}, | ||
{"code": "WV", "name": "West Virginia"}, | ||
{"code": "WI", "name": "Wisconsin"}, | ||
{"code": "WY", "name": "Wyoming"}, | ||
{"code": "DC", "name": "District of Columbia"}, | ||
{"code": "AS", "name": "American Samoa"}, | ||
{"code": "GU", "name": "Guam"}, | ||
{"code": "MP", "name": "Northern Mariana Islands"}, | ||
{"code": "PR", "name": "Puerto Rico"}, | ||
{"code": "UM", "name": "United States Minor Outlying Islands"}, | ||
{"code": "VI", "name": "Virgin Islands, U.S."}, | ||
] | ||
|
||
federal_regulator_seed = [ | ||
{"id": "FCA", "name": "Farm Credit Administration"}, | ||
{"id": "FDIC", "name": "Federal Deposit Insurance Corporation"}, | ||
{"id": "FHFA", "name": "Federal Housing Finance Agency"}, | ||
{"id": "FRS", "name": "Federal Reserve System"}, | ||
{"id": "NCUA", "name": "National Credit Union Administration"}, | ||
{"id": "OCC", "name": "Office of the Comptroller of the Currency"}, | ||
{"id": "OTS", "name": "Office of Thrift Supervision (only valid until July 21, 2011)"}, | ||
] | ||
|
||
sbl_institution_type_seed = [ | ||
{"id": "1", "name": "Bank or savings association."}, | ||
{"id": "2", "name": "Minority depository institution."}, | ||
{"id": "3", "name": "Credit union."}, | ||
{"id": "4", "name": "Nondepository institution."}, | ||
{"id": "5", "name": "Community development financial institution (CDFI)."}, | ||
{"id": "6", "name": "Other nonprofit financial institution."}, | ||
{"id": "7", "name": "Farm Credit System institution."}, | ||
{"id": "8", "name": "Government lender."}, | ||
{"id": "9", "name": "Commercial finance company."}, | ||
{"id": "10", "name": "Equipment finance company."}, | ||
{"id": "11", "name": "Industrial loan company."}, | ||
{"id": "12", "name": "Online lender."}, | ||
{"id": "13", "name": "Other"}, | ||
] | ||
|
||
hmda_institution_type_seed = [ | ||
{"id": "1", "name": "National Bank (OCC supervised)"}, | ||
{"id": "2", "name": "State Member Bank (FRS Supervised)"}, | ||
{"id": "3", "name": "State non-member bank (FDIC supervised)"}, | ||
{"id": "4", "name": "State Chartered Thrift (FDIC supervised)"}, | ||
{"id": "5", "name": "Federal Chartered Thrift (OCC supervised)"}, | ||
{"id": "6", "name": "Credit Union (NCUA supervised)"}, | ||
{"id": "7", "name": "Federal Branch or Agency of Foreign Banking Organization (FBO)"}, | ||
{"id": "8", "name": "Branch or Agency of FBO (FRS supervised)"}, | ||
{"id": "9", "name": "MBS of national Bank (OCC supervised)"}, | ||
{"id": "10", "name": "MBS of state member bank (FRS supervised)"}, | ||
{"id": "11", "name": "MBS of state non-member bank (FDIC supervised)"}, | ||
{"id": "12", "name": "MBS of Bank Holding Company (BHC) (FRS supervised)"}, | ||
{"id": "13", "name": "MBS of credit union (NCUA supervised)"}, | ||
{"id": "14", "name": "independent MBS, no depository affiliation"}, | ||
{"id": "15", "name": "MBS of Savings and Loan Holding Co"}, | ||
{"id": "16", "name": "MBS of state chartered Thrift"}, | ||
{"id": "17", "name": "MBS of federally chartered thrift (OCC supervised)"}, | ||
{"id": "18", "name": "Affiliate of depository institution. MBS is in the same ownership org as a depository."}, | ||
] | ||
|
||
""" | ||
END seed data for the lookup tables: | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
"""Feed Federal Regulator table | ||
"""Seed Federal Regulator table | ||
|
||
Revision ID: 26a742d97ad9 | ||
Revises: 7b6ff51002b5 | ||
|
@@ -7,8 +7,8 @@ | |
""" | ||
from typing import Sequence, Union | ||
from alembic import op | ||
from db_revisions.utils import get_feed_data_from_file | ||
from entities.models import FederalRegulatorDao | ||
from db_revisions.seed import federal_regulator_seed | ||
|
||
|
||
# revision identifiers, used by Alembic. | ||
|
@@ -19,9 +19,7 @@ | |
|
||
|
||
def upgrade() -> None: | ||
data = get_feed_data_from_file("federal_regulator") | ||
|
||
op.bulk_insert(FederalRegulatorDao.__table__, data) | ||
op.bulk_insert(FederalRegulatorDao.__table__, federal_regulator_seed) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we talked about keeping the table names hardcoded? same kind of data integrity concerns where as we iterate, if we decided to switch the table for the dao, the new script would succeed in the old environment at creating the new table; but it would fail in the new environment as the table would already exist after running the previous version table creation script. |
||
|
||
def downgrade() -> None: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if this was discussed already in the other PRs or with Le, but should the individual feed lists go into their corresponding migration scripts to keep the seed data with the creation of the table the data is going into? The config class seems to be specific to env configurations we pull from either env vars or .env files. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can move it inside its individual scripts. That's the way I initially had it, but then I needed to access the same data for testing. So, I wanted to have them just in one place. I can go ahead and move it inside the individual scripts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha, thought it might be that because of the comment in there. It's great that the tests check that the expected seed data is in tables so hopefully the tests can access the lists in the migrations (which is a little annoying to import each especially since the module names are horrific) or at the worst copy the list to the test testing the table. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should still have them inside the individual scripts for data integrity, considering the small data set. For testing purposes, you can test just a small sample of the data; or create test resources. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lchen-2101 Got it, yeah makes more sense. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We run into the same problem having this file rather than having the csv's. These should live within the scripts themselves; the purpose of the alembic scripts with versions is that each time the upgrade is ran, the same data is populated; we have decide to update the seed data, and we run the script in a different environment, then the 2 environments' data would become out of sync. Let's have these data within each script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if it was a big dataset, we should do the csv route, but version the file appropriately. i.e.
address_state_7b6ff51002b5.csv
; but since these are small, keeping them within the alembic script itself should be fine.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.