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

Fix: idempotent setup and teardown #104

Merged
merged 4 commits into from
Aug 3, 2022

Conversation

angela-tran
Copy link
Member

Closes #103

@angela-tran angela-tran added this to the Courtesy Cards milestone Aug 3, 2022
@angela-tran angela-tran self-assigned this Aug 3, 2022
@angela-tran angela-tran requested a review from a team as a code owner August 3, 2022 14:55
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

Nice find and nice simple fix 👍

In setup.py, should there be one more check to see if, when the table already exists, it still needs users to be loaded? Not sure if that's a use case that matters...

@angela-tran
Copy link
Member Author

angela-tran commented Aug 3, 2022

In setup.py, should there be one more check to see if, when the table already exists, it still needs users to be loaded? Not sure if that's a use case that matters...

I could see a case where maybe the IMPORT_FILE_PATH was wrong, and so the table was created but no data actually went into it. I'll add this check 👍

I think I'll also add a print statement for the case where data wasn't loaded.

@angela-tran angela-tran requested a review from thekaveman August 3, 2022 18:18
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

Looks good!

Maybe a follow-up: figure out how to get the Python logging framework configured and replace the print statements with more appropriate log-level statements.

@angela-tran angela-tran merged commit 9a32478 into main Aug 3, 2022
@angela-tran angela-tran deleted the fix/idempotent-setup-and-teardown branch August 3, 2022 22:08
@angela-tran angela-tran mentioned this pull request Nov 14, 2022
10 tasks
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.

Make database setup and teardown idempotent
2 participants