-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat!: support PostgreSQL as optional storage backend #116
Conversation
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.
Overall looks great!
if not click.confirm("Are you sure you want to overwrite existing data?"): | ||
click.get_current_context().exit() | ||
if not data_url: | ||
data_url = os.environ.get("DISEASE_NORM_REMOTE_DB_URL") |
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.
Are you working on a similar docs issue as you did in gene-normalizer? If so, it would be good to add DISEASE_NORM_REMOTE_DB_URL
to that issue
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.
Yeah, I want to do one more iteration on the gene docs and then I'll start building out comparable stuff with disease and therapy
@jsstevenson It also looks like there's some conflicts that need resolved before merging |
.github/workflows/github-actions.yml
Outdated
|
||
- name: Build local DynamoDB server | ||
- name: Build local DynamoDB | ||
if: ${{ env.GENE_NORM_DB_URL == 'http://localhost:8000' }} |
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.
if: ${{ env.GENE_NORM_DB_URL == 'http://localhost:8000' }} | |
if: ${{ env.DISEASE_NORM_DB_URL == 'http://localhost:8000' }} |
.github/workflows/github-actions.yml
Outdated
./tests/unit/dynamodb_build.bash | ||
|
||
- name: Install DynamoDB dependencies | ||
if: ${{ env.GENE_NORM_DB_URL == 'http://localhost:8000' }} |
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.
if: ${{ env.GENE_NORM_DB_URL == 'http://localhost:8000' }} | |
if: ${{ env.DISEASE_NORM_DB_URL == 'http://localhost:8000' }} |
.github/workflows/github-actions.yml
Outdated
run: python3 -m pip install ".[etl,test]" | ||
|
||
- name: Install PG dependencies | ||
if: ${{ env.GENE_NORM_DB_URL != 'http://localhost:8000' }} |
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.
if: ${{ env.GENE_NORM_DB_URL != 'http://localhost:8000' }} | |
if: ${{ env.DISEASE_NORM_DB_URL != 'http://localhost:8000' }} |
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.
f
Finalizes a dev release implementing an optional Postgres DB backend to use in TheraPy CI. Once tests are passing for everything, there will be another full release.
Otherwise, everything is basically the same as the gene PR. I've noticed that all three normalizers have slightly different query method structure, which would be good to fix up at some point, but not pressing rn.