chore: Remove Server Tasks #3592
Conversation
| @@ -67,7 +73,7 @@ class Group(SqlAlchemyBase, BaseMixins): | |||
| webhooks: Mapped[list[GroupWebhooksModel]] = orm.relationship(GroupWebhooksModel, **common_args) | |||
| recipe_actions: Mapped[list["GroupRecipeAction"]] = orm.relationship("GroupRecipeAction", **common_args) | |||
| cookbooks: Mapped[list[CookBook]] = orm.relationship(CookBook, **common_args) | |||
| server_tasks: Mapped[list[ServerTaskModel]] = orm.relationship(ServerTaskModel, **common_args) | |||
| server_tasks: Mapped[list["ServerTaskModel"]] = orm.relationship("ServerTaskModel", **common_args) | |||
There was a problem hiding this comment.
Why did this need to change?
(I don't have a problem with it, just not sure why the change is being made)
There was a problem hiding this comment.
Ah yeah, that's because I removed the line by accident then put it back (and accidentally did it slightly differently). Specifically it's different because of how imports work with SQLAlchemy: I could've put it back how it was, but I also moved the import to typing only (which means it's not actually imported at runtime):
# earlier in the file
if TYPE_CHECKING:
...
from ..server.task import ServerTaskModelThis is a better way of handling imports in this scenario since it's slightly more performant and avoids circular imports (though that's not actually an issue here). In theory we should replace all of our SQLAlchemy imports to do this, but the performance benefit is negligible so it's not worth the effort.
TL;DR it's a totally unnecessary change I did by accident, but I'm inclined to leave it because it's technically better anyway
There was a problem hiding this comment.
Or not at all and prod breaks. Cool
#3630
There was a problem hiding this comment.
At least we had fun 😅
michael-genson
merged commit 61becdb
into
mealie-recipes:mealie-next
What type of PR is this?
(REQUIRED)
What this PR does / why we need it:
(REQUIRED)
Removes all mentions of the old server tasks API, except for the database model. I left the database model because otherwise Alembic migrations are going to whine about it, though I added a comment.
Which issue(s) this PR fixes:
(REQUIRED)
Mentioned in #3442