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

Use Flask-Migrate in Makefile #1132

Merged
merged 1 commit into from
Nov 6, 2024
Merged

Conversation

michplunkett
Copy link
Collaborator

@michplunkett michplunkett commented Oct 29, 2024

Description of Changes

If we are using Flask-Migrate commands everywhere else, we shouldn't make an exception for the Makefile.

Tests and Linting

  • This branch is up-to-date with the develop branch.
  • pytest passes on my local development environment.
  • pre-commit passes on my local development environment.
  • Verify make create_db works correctly.
(env) michaelp@MacBook-Air-18 OpenOversight % make create_db
docker compose build
WARN[0000] The "AWS_DEFAULT_REGION" variable is not set. Defaulting to a blank string.
WARN[0000] The "AWS_SECRET_ACCESS_KEY" variable is not set. Defaulting to a blank string.
WARN[0000] The "AWS_ACCESS_KEY_ID" variable is not set. Defaulting to a blank string.
WARN[0000] The "S3_BUCKET_NAME" variable is not set. Defaulting to a blank string.
WARN[0000] The "APPROVE_REGISTRATIONS" variable is not set. Defaulting to a blank string.
[+] Building 0.6s (25/25) FINISHED                                                                                                                                                         docker:desktop-linux
 ...
Postgres is up
## Creating database
docker compose run --rm web flask db stamp head
...
INFO  [sqlalchemy.engine.Engine] COMMIT
INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO  [alembic.runtime.migration] Will assume transactional DDL.
(env) michaelp@MacBook-Air-18 OpenOversight %

@michplunkett michplunkett self-assigned this Oct 29, 2024
@michplunkett michplunkett changed the title Use Flask-Migrate not Alembic Use Flask-Migrate in Makefile Oct 29, 2024
@@ -29,7 +29,7 @@ create_db: start
done
@echo "Postgres is up"
## Creating database
docker compose run --rm web alembic --config=./OpenOversight/migrations/alembic.ini stamp head
docker compose run --rm web flask db stamp head
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't specifically reference Alembic anywhere else in the project. This makes the Makefile consistent with the rest of the app.

Copy link
Collaborator

@sea-kelp sea-kelp 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 to me! It might also be worth adding a make target for Flask-Migrate:
https://github.com/OrcaCollective/OpenOversight/blob/6d5c7b08b24a7669218d78b780f6547132d89351/justfile#L95-L97

@michplunkett
Copy link
Collaborator Author

I'll take a look and see what that looks like for Make.

@michplunkett
Copy link
Collaborator Author

Looks like that functionality is a little more complicated with Makefiles: https://stackoverflow.com/a/45003119

@michplunkett michplunkett merged commit 0c430e7 into develop Nov 6, 2024
3 checks passed
@michplunkett michplunkett deleted the switch-to-full-flask-migrate branch November 6, 2024 22:12
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