feat: create database models #6

greyli · 2023-12-18T14:48:07Z

Add db models
Add commands to create/drop tables
Fix pdm serve command

yuxiaoy1 · 2023-12-18T15:47:53Z

2 questions:

why not following SQLAlchemy 2.0 code style?
commands can be a flask Blueprint and use app.register_blueprint(commands) directly, so no reigster_commands needed. and I suggest to rename directory views to blueprints for better understanding, like from app.blueprints.commands import commands.

greyli · 2023-12-18T16:13:43Z

why not following SQLAlchemy 2.0 code style?

Good suggestion! I have to admit I have never writen the 2.0 style model class. I will update it later. Or maybe we could merge this first since it blocks others tasks.

commands can be a flask Blueprint and use app.register_blueprint(commands) directly, so no reigster_commands needed. and I suggest to rename directory views to blueprints for better understanding, like from app.blueprints.commands import commands.

I was try to keep the "views" folder for "view functions", other configuration for the app goes to the "core" folder.

@frostming What do you think?

yuxiaoy1 · 2023-12-18T16:36:55Z

The module main, admin are all blueprints, while you can add views in that module. In larger applicaiton, you can optionally structure like this: blueprints/main/__init__.py, blueprints/main/views.py( or .../routes.py), which the naming convention would be more accurate.

frostming · 2023-12-19T01:16:40Z

.flaskenv

@@ -1,2 +0,0 @@
-FLASK_DEBUG=1
-FLASK_APP=backend/app.py


Sorry, I didn't notice it is pointing at backend subfolder, then I prefer to keep it at the top level but fixing the tool.pdm.scripts instead.

frostming · 2023-12-19T01:19:19Z

commands can be a flask Blueprint and use app.register_blueprint(commands) directly

I am confused by this, how can commands be a blueprint but it isn't?

The module main, admin are all blueprints, while you can add views in that module. In larger applicaiton, you can optionally structure like this: blueprints/main/__init__.py, blueprints/main/views.py( or .../routes.py), which the naming convention would be more accurate.

I agree with this restructure.

yuxiaoy1 · 2023-12-19T03:49:52Z

You can create a commands blueprint and set the cli_group argument to None, then you are free to add commands with @commands.cli.command as usual.

frostming · 2023-12-19T05:57:32Z

You can create a commands blueprint and set the cli_group argument to None, then you are free to add commands with @commands.cli.command as usual.

I see, but the naming is not right. They are app-level commands that do not belong to any blueprint.

yuxiaoy1 · 2023-12-19T06:43:21Z

It's app level, the key point is cli_group argument, so you can use flask xxx, otherwise you need to use flask commands xxx.

zhaoblake · 2023-12-19T07:14:37Z

Any updates on this PR? I am planning to build APIs, but I am blocked because of this. @frostming @greyli

backend/bamboo/models.py

- Add db models - Add commands to create/drop tables - Fix pdm serve command

greyli · 2023-12-19T14:02:07Z

Fixed all issues and migrated to SQLAlchemy 2.x code style. @frostming @yuxiaoy1 Please help to review.

backend/bamboo/models.py

yuxiaoy1 · 2023-12-19T14:17:12Z

backend/bamboo/models.py

+
+
+class User(db.Model):
+    id: Mapped[int] = mapped_column(Integer, primary_key=True)


SQLAlchemy 2.x automatically set the data type with type hint, so the declaration can be simplified to id: Mapped[int] = mapped_column(primary_key=True) name: Mapped[str] username: Mapped[str] ...

yuxiaoy1 · 2023-12-19T14:19:48Z

backend/bamboo/models.py

+    password_hash: Mapped[str] = mapped_column(String)
+    biology: Mapped[str] = mapped_column(String)
+    introduction: Mapped[str] = mapped_column(String)
+    created_at: Mapped[datetime] = mapped_column(DateTime, default=datetime.utcnow)


According to https://blog.miguelgrinberg.com/post/it-s-time-for-a-change-datetime-utcnow-is-now-deprecated, it's better to use default=lambda: datetime.now(timezone.utc)

Updated and used datetime.UTC to replace timezone.utc.

yuxiaoy1 · 2023-12-19T14:20:21Z

backend/bamboo/models.py

+    introduction: Mapped[str] = mapped_column(String)
+    created_at: Mapped[datetime] = mapped_column(DateTime, default=datetime.utcnow)
+    updated_at: Mapped[datetime] = mapped_column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow)
+    active: Mapped[str] = mapped_column(Boolean)


This should be Mapped[bool], and remove the mapped_column

yuxiaoy1 · 2023-12-19T14:22:45Z

backend/bamboo/core/extensions.py

+from sqlalchemy.orm import DeclarativeBase
+
+
+class Base(DeclarativeBase):


This seems to be unnecessary.

It's recommended by the Flask-SQLAlchemy docs.

Right, but I have checked there is no difference w/ or w/o it, I think the docs need get updates.

yuxiaoy1 · 2023-12-19T14:28:52Z

backend/bamboo/models.py

+    role_id: Mapped[int] = mapped_column(Integer, ForeignKey('role.id'))
+
+    profile_image = relationship('Media', foreign_keys=[profile_image_id])
+    role = relationship('Role', back_populates='users')


Use WriteOnlyMapped for relationship: role: WriteOnlyMapped['Role'] = relationship(back_populates='users')
All relationship could be updated as above.

yuxiaoy1 · 2023-12-19T14:34:45Z

backend/bamboo/models.py

+    name: Mapped[str] = mapped_column(String)
+    username: Mapped[str] = mapped_column(String)
+    password_hash: Mapped[str] = mapped_column(String)
+    biology: Mapped[str] = mapped_column(String)


This should be biography? or just bio.
And this could be null so add optional type hint: bio: Mapped[Optional[str]], or it would get not null constraint error using sqlite.
All field optional need updates as above.

Changed to bio and updated optional fields.

greyli · 2023-12-19T15:49:08Z

@yuxiaoy1 Thank you for the thorough review. I have updated the code based on your suggestions.

greyli · 2023-12-19T15:55:20Z

The new code throws:

➜  bamboo git:(db-models) pdm create-tables
Traceback (most recent call last):
  File "/home/greyli/bamboo/.venv/bin/flask", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/greyli/bamboo/.venv/lib/python3.12/site-packages/flask/cli.py", line 1064, in main
    cli.main()
  File "/home/greyli/bamboo/.venv/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/greyli/bamboo/.venv/lib/python3.12/site-packages/click/core.py", line 1682, in invoke
    cmd_name, cmd, args = self.resolve_command(ctx, args)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/greyli/bamboo/.venv/lib/python3.12/site-packages/click/core.py", line 1729, in resolve_command
    cmd = self.get_command(ctx, cmd_name)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/greyli/bamboo/.venv/lib/python3.12/site-packages/flask/cli.py", line 579, in get_command
    app = info.load_app()
          ^^^^^^^^^^^^^^^
  File "/home/greyli/bamboo/.venv/lib/python3.12/site-packages/flask/cli.py", line 309, in load_app
    app = locate_app(import_name, name)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/greyli/bamboo/.venv/lib/python3.12/site-packages/flask/cli.py", line 219, in locate_app
    __import__(module_name)
  File "/home/greyli/bamboo/backend/app.py", line 8, in <module>
    from bamboo import create_app  # noqa
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/greyli/bamboo/backend/bamboo/__init__.py", line 9, in <module>
    from bamboo.core.commands import register_commands
  File "/home/greyli/bamboo/backend/bamboo/core/commands.py", line 4, in <module>
    from bamboo.models import *
  File "/home/greyli/bamboo/backend/bamboo/models.py", line 12, in <module>
    class User(db.Model):
  File "/home/greyli/bamboo/backend/bamboo/models.py", line 25, in User
    profile_image = WriteOnlyMapped['Media'] = relationship(foreign_keys=[profile_image_id])
                    ~~~~~~~~~~~~~~~^^^^^^^^^
TypeError: 'type' object does not support item assignment

I think it's better to move the migration to a separate pull request. Removed the last commit.

yuxiaoy1 · 2023-12-19T16:06:10Z

Ok I think we can pull request later.

frostming · 2023-12-20T00:49:28Z

backend/bamboo/models.py

+class Site(db.Model):
+    __tablename__ = 'site'
+    id = Column(Integer, primary_key=True)
+    config = Column(Text)


Does sqlite3 support JSON natively? It seems so for the latest versions. And both MySQL and PostgreSQL support JSON. Maybe we can update it.

Updated. SQLite 3.9 and later supports the JSON field.

frostming

Okay, just a minor comment and I think we should merge this one to unblock the other tasks.

greyli changed the title ~~Create database models~~ feat: create database models Dec 18, 2023

greyli force-pushed the db-models branch from 9af6431 to 9d5d3ea Compare December 18, 2023 15:48

frostming reviewed Dec 19, 2023

View reviewed changes

greyli force-pushed the db-models branch from 9d5d3ea to 902d0da Compare December 19, 2023 01:20

frostming reviewed Dec 19, 2023

View reviewed changes

backend/bamboo/models.py Outdated Show resolved Hide resolved

frostming reviewed Dec 19, 2023

View reviewed changes

backend/bamboo/models.py Outdated Show resolved Hide resolved

frostming reviewed Dec 19, 2023

View reviewed changes

backend/bamboo/models.py Outdated Show resolved Hide resolved

feat: create database models

eb724e3

- Add db models - Add commands to create/drop tables - Fix pdm serve command

greyli force-pushed the db-models branch from 902d0da to 1d8b752 Compare December 19, 2023 13:31

greyli force-pushed the db-models branch from 1d8b752 to f93a28d Compare December 19, 2023 14:09

yuxiaoy1 suggested changes Dec 19, 2023

View reviewed changes

greyli force-pushed the db-models branch from f93a28d to ffac5a7 Compare December 19, 2023 15:44

greyli force-pushed the db-models branch from ffac5a7 to 9c652f2 Compare December 19, 2023 15:54

frostming reviewed Dec 20, 2023

View reviewed changes

frostming approved these changes Dec 20, 2023

View reviewed changes

refactor: add schema files and rename views to blueprints

47ad0c1

greyli force-pushed the db-models branch from 9c652f2 to 47ad0c1 Compare December 20, 2023 01:10

Merge branch 'main' into db-models

04f5964

frostming merged commit c46ce08 into bamboo-cms:main Dec 20, 2023
2 checks passed

greyli deleted the db-models branch December 20, 2023 04:10

frostming mentioned this pull request Dec 20, 2023

Migrate models to SQLAlchemy v2 #11

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create database models #6

feat: create database models #6

greyli commented Dec 18, 2023

yuxiaoy1 commented Dec 18, 2023

greyli commented Dec 18, 2023

yuxiaoy1 commented Dec 18, 2023

frostming Dec 19, 2023

frostming commented Dec 19, 2023

yuxiaoy1 commented Dec 19, 2023

frostming commented Dec 19, 2023 •

edited

yuxiaoy1 commented Dec 19, 2023

zhaoblake commented Dec 19, 2023

greyli commented Dec 19, 2023 •

edited

yuxiaoy1 Dec 19, 2023

greyli Dec 19, 2023

yuxiaoy1 Dec 19, 2023

greyli Dec 19, 2023

yuxiaoy1 Dec 19, 2023

greyli Dec 19, 2023

yuxiaoy1 Dec 19, 2023

greyli Dec 19, 2023

yuxiaoy1 Dec 19, 2023

yuxiaoy1 Dec 19, 2023

greyli Dec 19, 2023

yuxiaoy1 Dec 19, 2023 •

edited

greyli Dec 19, 2023

greyli commented Dec 19, 2023

greyli commented Dec 19, 2023

yuxiaoy1 commented Dec 19, 2023

frostming Dec 20, 2023

greyli Dec 20, 2023

frostming left a comment



		class User(db.Model):
		id: Mapped[int] = mapped_column(Integer, primary_key=True)

		from sqlalchemy.orm import DeclarativeBase


		class Base(DeclarativeBase):

		@@ -1,2 +0,0 @@
		FLASK_DEBUG=1
		FLASK_APP=backend/app.py

feat: create database models #6

feat: create database models #6

Conversation

greyli commented Dec 18, 2023

yuxiaoy1 commented Dec 18, 2023

greyli commented Dec 18, 2023

yuxiaoy1 commented Dec 18, 2023

Choose a reason for hiding this comment

frostming commented Dec 19, 2023

yuxiaoy1 commented Dec 19, 2023

frostming commented Dec 19, 2023 • edited

yuxiaoy1 commented Dec 19, 2023

zhaoblake commented Dec 19, 2023

greyli commented Dec 19, 2023 • edited

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

yuxiaoy1 Dec 19, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greyli commented Dec 19, 2023

greyli commented Dec 19, 2023

yuxiaoy1 commented Dec 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frostming left a comment

Choose a reason for hiding this comment

frostming commented Dec 19, 2023 •

edited

greyli commented Dec 19, 2023 •

edited

yuxiaoy1 Dec 19, 2023 •

edited