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

Postgres woes #579

Closed
micahflee opened this issue Sep 11, 2024 · 3 comments
Closed

Postgres woes #579

micahflee opened this issue Sep 11, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@micahflee
Copy link
Collaborator

After the merging of #572, there are a few issues:

Hush Line is deployed via a docker image, and the Dockerfile includes running tests to make sure everything works before generating it. Before it would run the tests using a new sqlite db, but now that we're only using postgres, that fails, and thus the docker image cannot be built: https://github.com/scidsg/hushline/actions/runs/10806155886/job/29974447684

Also, we have a dev deploy workflow: https://github.com/scidsg/hushline/blob/main/.github/workflows/dev_deploy.yml

If we add the "deploy" label to a PR, it spins up a dev environment in DigitalOcean using some terraform magic. Before, this dev environment would use a local sqlite db (since it was just temporary and we didn't care), but now we need to refactor the infra terraform code to support also spinning up a dev postgres container and using that.

@micahflee micahflee added the bug Something isn't working label Sep 11, 2024
@brassy-endomorph
Copy link
Collaborator

brassy-endomorph commented Sep 11, 2024

Test should not be run in the container as a build step. That is an anti-pattern. If you want something that tests the final artifact, you need an end-to-end test suite. It might be possible to set up pytest and flask to target a remote server (as in spin up the container pointing at postgres, run the tests against it), but I haven't seen that done before, so I'm initially doubtful.

@brassy-endomorph
Copy link
Collaborator

brassy-endomorph commented Sep 11, 2024

If we want a smoke test for the final container, I'd make a dir called e2e-test and put a couple of tests in using requests instead of the Flask test client. Then we run those with pytest like it's a normal test suite.

So I'd say for now yank the tests from the container. We barely have normal test coverage as it is, so removing that doesn't change much. We're also small and nimble, so quickly rolling out a patch in the even of a broken prod deploy shouldn't be an issue.

Basically, better to either yank it or commit to doing it right than some kinda shoehorn to make the tests in a RUN step work.

@micahflee
Copy link
Collaborator Author

PR to yank the tests #580

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants