Switch to SQLModel Database stuff #93

jontyms · 2024-05-14T15:12:26Z

TODO:

docker-compose-tests.yml

app/util/forms.py

Helithumper · 2024-05-16T02:47:27Z

We'll need some sort of migration system for at least up-migrations on the SQL DB

Helithumper · 2024-05-16T02:40:03Z

Dockerfile

@@ -22,7 +22,7 @@ RUN mv bws /usr/local/bin
 RUN rm -r /tmp/

 # Install the dependencies
-RUN pip install --no-cache-dir -r requirements.txt
+RUN --mount=type=cache,target=/root/.cache/pip pip install -r requirements.txt


Unsure if this is a good idea or not but haven't seen it used before

This create a pip cache that is stored outside of the docker container, speeding up rebuilds
https://pythonspeed.com/articles/docker-cache-pip-downloads/

Just looking at random repo and came across this. If you want a faster build, you can always setup CI pipeline with GH Action and store it on PKG registry, then fetch it on the server.

Helithumper · 2024-05-16T02:40:24Z

Dockerfile-testing

+# Clean up
+RUN apt-get clean && rm -rf /var/lib/apt/lists/*
+
+ADD https://github.com/bitwarden/sdk/releases/download/bws-v0.4.0/bws-x86_64-unknown-linux-gnu-0.4.0.zip /tmp


Is this the official installation method for the bitwarden SDK?

Yes https://bitwarden.com/help/secrets-manager-cli/

I didn't see they had an sdk before might switch to that https://bitwarden.com/help/secrets-manager-sdk/

Helithumper · 2024-05-16T02:40:48Z

app/main.py

-
+    statement = select(UserModel).where(UserModel.discord_id == discordData["id"])
+    user = session.exec(statement).one_or_none()
+    print(user)


Replace with logger usage

Helithumper · 2024-05-16T02:41:23Z

app/main.py

-            UpdateExpression="SET discord = :discord",
-            ExpressionAttributeValues={":discord": full_data["discord"]},
-        )
+            "color": discordData.get("accent_color"),


Should these default to something other than None?

Helithumper · 2024-05-16T02:42:01Z

app/main.py

+    # Get data from SqlModel
+    statement = (
+        select(UserModel)
+        .where(UserModel.id == user_jwt["id"])


Handling of KeyError?

Should not occur @Authentication.member will fail if the user has an invalid JWT

Helithumper · 2024-05-16T02:42:32Z

app/models/user.py

+
+
+class DiscordModel(SQLModel, table=True):
+    id: Optional[int] = Field(default=None, primary_key=True)


Is it a good idea to make all these fields nullable? Why not just make the reference to this table nullable instead?

Probably not

Helithumper · 2024-05-16T02:43:16Z

app/models/user.py

+
+
+
+class UserModel(SQLModel, table=True):


Should almost all of these fields be nullable?

I will review, but most of the values are unknown when we first write the user to the DB before they do the onboarding forms

Helithumper · 2024-05-16T02:44:37Z

app/routes/api.py

+router = APIRouter(prefix="/api", tags=["API"], responses=Errors.basic_http())
+
+
+"""


Docstrings go under the function signature

Helithumper · 2024-05-16T02:45:10Z

app/routes/api.py

+
+    # Update the ethics form with new values
+    validated_data = apply_fuzzy_parsing(ethics_form_data.model_dump(exclude_unset=True), EthicsFormModel)
+    print(validated_data.dict())


Helithumper · 2024-05-16T02:46:27Z

app/util/database.py

+from sqlmodel import Session, SQLModel, create_engine
+from sqlmodel.pool import StaticPool
+
+DATABASE_URL = "sqlite:///:memory:"


Read from ENVVAR w/ Defaults

Helithumper · 2024-05-16T02:48:19Z

Did a quick sweep with some notes, Overall a good first step but needs some polish 🥳

Helithumper · 2024-05-16T02:48:50Z

Dockerfile

@@ -22,7 +22,7 @@ RUN mv bws /usr/local/bin
 RUN rm -r /tmp/

 # Install the dependencies
-RUN pip install --no-cache-dir -r requirements.txt
+RUN --mount=type=cache,target=/root/.cache/pip pip install -r requirements.txt

 # Copy the application code to the container
 COPY . .


Include a .Dockerignore for files that don't need to be in the container

Helithumper · 2024-05-16T02:49:10Z

Dockerfile-testing

+WORKDIR /app
+
+# Copy the requirements file to the container
+COPY requirements.txt .


Move this further down, any changes to requirements will cause the apt execution to restart

Helithumper · 2024-05-16T02:50:50Z

docker-compose.yml

@@ -8,6 +8,7 @@ services:
      - 8000:8000
    volumes:
      - ./config/options.yml:/app/config/options.yml
+      - ./database/:/app/database


Nit: usually this is called ./data

jontyms · 2024-05-16T03:50:44Z

We'll need some sort of migration system for at least up-migrations on the SQL DB

https://testdriven.io/blog/fastapi-sqlmodel/

most likely we will use Alembic

Helithumper · 2024-05-16T04:16:23Z

Alembic's fine, that works

app/util/forms.py

jontyms added 7 commits May 13, 2024 22:05

initial tests

da1d679

WIP database

aee5aed

WIP database

f1d1f53

WIP database

630de0c

WIP Database

563bbca

WIP database

4941cbc

WIP tests

e8f8d46

github-advanced-security bot found potential problems May 15, 2024

View reviewed changes

docker-compose-tests.yml Fixed Show fixed Hide fixed

docker-compose-tests.yml Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems May 15, 2024

View reviewed changes

app/util/forms.py Fixed Show fixed Hide fixed

WIP tests

03969cf

Helithumper reviewed May 16, 2024

View reviewed changes

WIP tests

31259c1

jontyms added 4 commits May 19, 2024 16:47

forms broken

272671e

Everything Works?

7d0070b

Everything Works?

81c3e51

Everything Works?

5093ef0

github-advanced-security bot found potential problems May 27, 2024

View reviewed changes

app/util/forms.py Dismissed Show dismissed Hide dismissed

fixed file inclusion

1b0604d

github-advanced-security bot found potential problems May 27, 2024

View reviewed changes

app/util/forms.py Dismissed Show dismissed Hide dismissed

jontyms marked this pull request as ready for review May 27, 2024 17:28

stuff works

6dbc0d8

jontyms changed the title ~~WIP - Switch to SQLModel Database stuff~~ Switch to SQLModel Database stuff May 27, 2024

jontyms added 3 commits May 27, 2024 14:23

Fixed Doc Strings

9ed938d

Ruff Fixes

9f8fe3f

Ruff Format

0cb90e6

Removed iosort

b1d25bf

jontyms merged commit 7522882 into HackUCF:main May 27, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to SQLModel Database stuff #93

Switch to SQLModel Database stuff #93

jontyms commented May 14, 2024 •

edited

Loading

Helithumper commented May 16, 2024

Helithumper May 16, 2024

jontyms May 16, 2024

zhiyan114 May 30, 2024

Helithumper May 16, 2024

jontyms May 16, 2024

jontyms May 16, 2024

Helithumper May 16, 2024

Helithumper May 16, 2024

Helithumper May 16, 2024

jontyms May 16, 2024

Helithumper May 16, 2024

jontyms May 16, 2024

Helithumper May 16, 2024

jontyms May 16, 2024

Helithumper May 16, 2024

Helithumper May 16, 2024

Helithumper May 16, 2024

Helithumper commented May 16, 2024

Helithumper May 16, 2024

jontyms May 16, 2024

Helithumper May 16, 2024

jontyms May 16, 2024

Helithumper May 16, 2024

jontyms May 16, 2024

jontyms commented May 16, 2024

Helithumper commented May 16, 2024



		class DiscordModel(SQLModel, table=True):
		id: Optional[int] = Field(default=None, primary_key=True)

		router = APIRouter(prefix="/api", tags=["API"], responses=Errors.basic_http())


		"""

Switch to SQLModel Database stuff #93

Switch to SQLModel Database stuff #93

Conversation

jontyms commented May 14, 2024 • edited Loading

Helithumper commented May 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Helithumper commented May 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jontyms commented May 16, 2024

Helithumper commented May 16, 2024

jontyms commented May 14, 2024 •

edited

Loading