feat: Add ORM for organization model #1914
Conversation
mattzh72
changed the title
There was a problem hiding this comment.
I've made some comments that might be considered blocking but I don't want to block a merge once you feel it's ready. I really hate GH no longer having a "approved once these 3 things are done" option :|
Generally a good start to getting the code migrated into an object model - great stuff @mattzh72 ! glad to see you pulling this to life!
| def default(cls, db_session: "Session") -> "Organization": | ||
| """Get the default org, or create it if it doesn't exist.""" | ||
| try: | ||
| return db_session.query(cls).filter(cls.name == "Default Organization").one() |
There was a problem hiding this comment.
if you've got the default org ID available in constants it would probably be safer to use that - since names aren't unique someone could name a thing "Default Organization" and you have trouble
If you want names to be unique on the other hand you can add a UniqueConstraint to the table, but using the id lookup is cleaner IMO
| @@ -50,10 +49,10 @@ def delete_org( | |||
| ): | |||
| # TODO make a soft deletion, instead of a hard deletion | |||
| try: | |||
There was a problem hiding this comment.
you can just use Organization.default(db_session) here or do that one layer down in sever (whereever you land the db_session)
| DEFAULT_USER_ID, | ||
| DEFAULT_USER_NAME, | ||
| ) | ||
| from letta.constants import DEFAULT_ORG_ID, DEFAULT_USER_ID, DEFAULT_USER_NAME |
There was a problem hiding this comment.
once you have a user model, I'd create a similar User.default(db_session) to the Org version and move all that default logic to the model.
| org = Organization(name=DEFAULT_ORG_NAME, id=DEFAULT_ORG_ID) | ||
| self.ms.create_organization(org) | ||
| try: | ||
| self.organization_manager.get_organization_by_id(DEFAULT_ORG_ID) |
There was a problem hiding this comment.
this can be Organization.default(db_session)
| def create_organization(self, name: Optional[str] = None) -> PydanticOrganization: | ||
| """Create a new organization. If a name is provided, it is used, otherwise, a random one is generated.""" | ||
| with self.session_maker() as session: | ||
| org = Organization(name=name if name else create_random_username()) |
There was a problem hiding this comment.
definitely want to move the default logic to the ORM model, defaults are set only once and as close to the db as possible - otherwise they show up anywhere/everywhere in the function chain and it's painful to debug "where the heck is this name coming from?!?!"
on the model this would be
name: Mapped[str] = mapped_column(doc="The display name of the organization.", default=create_random_username)| return org.to_pydantic() | ||
|
|
||
| def create_default_organization(self) -> PydanticOrganization: | ||
| """Create the default organization.""" |
There was a problem hiding this comment.
alternatively you could get the ORM default and do to_record so you're only setting default logic in one place
Description:
Sets up the standard ORM model for the codebase. This is quite a large change:
CommonSqlalchemyMetaMixinsWe borrow a lot of code from @norton120 , thank you for the guidance in writing this PR!
New files:
Changes to existing files:
OrganizationModeland related functionsOrganizationand add that to serverTesting:
Since this is a refactor, we ensure the existing tests pass and add a new unit test for organization creation.