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

Fastapi conversion #818

Merged
merged 13 commits into from
Oct 9, 2024
Merged

Fastapi conversion #818

merged 13 commits into from
Oct 9, 2024

Conversation

paulespinosa
Copy link
Member

@paulespinosa paulespinosa commented Oct 7, 2024

Related #789

What changes did you make?

  • Started with clean Alembic migration environment and added migration script to insert roles into the database.
  • Create backend startup scripts to pre-populate the database and moto server with test user accounts.
  • Updated backend Dockerfile
  • Run the API using a script instead of directly in order to ensure test user accounts are populated before starting the API.
  • Updated docker-compose
    • Added moto server as a container
    • Added pgAdmin4 as a container

Rationale behind the changes?

Testing done for these changes

  • Run docker compose on development machine and manually checked that all services are integrated.
HUU-docker-environment HUU-pgadmin4 HUU-motoserver

This commit updates alembic migrations, creates docker compose environment, and
creates test users for local development.
The create_groups_users.py program needed to get the db URL from the environment variable.
The example has keys and values that can be used directly during development.
In this commit, the frontend will still fail to build due to test errors.
However, that is being fixed in parallel and this will ensure that the frontend
will be able to run in a container when it does get fixed.
This commit includes basic documentation about pre-populated test user accounts.
By having the backend depend on pgadmin, the `db` container will transitively be
run too.
Updated the service URLs.
The backend startup scripts that pre-populated moto server was allowing
duplicate data to be added to moto. This happens when moto server is running
while the API is repeatedly started using the `entrypoint` script.
Copy link
Member

@tylerthome tylerthome left a comment

Choose a reason for hiding this comment

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

This is looking good to me, thanks for adding and updating the notes and docs!

Comment on lines +75 to +84
sql = 'INSERT INTO public.user (email, "firstName", "lastName", "roleId") VALUES (%s, %s, %s, %s) ON CONFLICT(email) DO NOTHING'
url = urlparse(os.environ['DATABASE_URL'])
with psycopg2.connect(database=url.path[1:],
user=url.username,
password=url.password,
host=url.hostname,
port=url.port) as db_conn:
with db_conn.cursor() as cur:
cur.executemany(sql, rows)
db_conn.commit()
Copy link
Member

Choose a reason for hiding this comment

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

Is the direct use of postgres bindings here for simplicity (in place of sqlalchemy) or is there a more important reason to be aware of? I'm good with this, just curious to see if we can make a note about it

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, for simplicity only. I hope someone at some time will build on this. This suffers from the issue of synchronizing the model from the app and this statement.

The backend entrypoint script has a check for "prod" as in production. In a
production environment, there would not be a need to create test users. So, this
commit moves towards the idea of having a production environment.
@johnwroge johnwroge merged commit e5886b7 into main Oct 9, 2024
0 of 2 checks passed
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.

3 participants