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

Schemas migrations #1179

Merged
merged 12 commits into from
Jan 11, 2024
Merged

Schemas migrations #1179

merged 12 commits into from
Jan 11, 2024

Conversation

aybruhm
Copy link
Member

@aybruhm aybruhm commented Jan 10, 2024

Description

What was done:

  1. Change referenced field key_name that were used with odmantic.

If you have a look at any document record in the app_variants collection, you will notice that the base key for any record (in the collection) has been renamed to bases and the config key has been renamed to configs. The reason why we have it this way is because we modified the field reference key_name while using odmantic. In order to migrate the records in our database and have beanie access the document field names, the record keys need to be renamed back to what is set in the model field. Otherwise, beanie would raise the error -> AttributeError: 'AppVariantDB' object has no attribute 'bases'

Example

i. Odmantic Model:

class OldAppVariantDB(Model):
    ....
    base: VariantBaseDB = Reference(key_name="bases")
    config_name: Optional[str]
    config: ConfigDB = Reference(key_name="configs")

The record in the above model would be saved as:

{
  "_id": {
    "$oid": "65a002fe5f8b1c3c46c56a66"
  },
  ......
  "bases": {...},
  "config_name": "default",
  "configs": {...}
  ......
}

ii. Beanie Document:

class AppVariantDB(Document):
    base: Link[VariantBaseDB]
    config_name: Optional[str]
    config: Link[ConfigDB]

The record in the below model would be saved as:

{
  "_id": {
    "$oid": "65a002fe5f8b1c3c46c56a66"
  },
  ......
  "base": {...},
  "config_name": "default",
  "config": {...},
  ......
}

Because our records are saved using odmantic, beanie would try to access base and config and would result in an attribute error (what exists instead is the key_name that was defined in odmantic reference, which in this case are: bases and configs).

  1. Move old evaluation and evaluation scenarios records to conform to new evaluation and evaluation scenarios schemas. (Kindly see code for comments)

Additional Information:

To migrate the changes, execute agenta-backend container in bash and run the following command:

beanie migrate --no-use-transaction -uri 'mongodb://username:password@mongo:27017' -db 'agenta_v2' -p agenta_backend/migrations/ --forward

To make use of the --no-use-transaction flag, kindly follow the guide here.

@aybruhm aybruhm requested a review from mmabrouk January 10, 2024 11:08
Copy link
Member

@mmabrouk mmabrouk left a comment

Choose a reason for hiding this comment

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

Thanks for the great work @aybruhm !
I added a couple of comments

@aybruhm aybruhm marked this pull request as ready for review January 11, 2024 15:33
@aybruhm aybruhm requested review from aakrem and mmabrouk January 11, 2024 15:39
Copy link
Collaborator

@aakrem aakrem left a comment

Choose a reason for hiding this comment

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

now that we fixed the issue of the initialisation can we simply import the schemas instead of duplicating them here ?

Copy link
Member

@mmabrouk mmabrouk left a comment

Choose a reason for hiding this comment

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

Great work Abram!

@aybruhm
Copy link
Member Author

aybruhm commented Jan 11, 2024

now that we fixed the issue of the initialisation can we simply import the schemas instead of duplicating them here ?

We can, but it's not a good idea. Simply because duplicating the models in the migration file allows for a snapshot of the model's structure at the time of the migration. This snapshot will then be used to perform the necessary data and schema migrations without having to rely on the current state of the models, which is bound to change.

@mmabrouk mmabrouk merged commit 6ce0592 into evaluations-in-backend Jan 11, 2024
2 of 3 checks passed
@mmabrouk mmabrouk deleted the schemas-migrations branch January 11, 2024 17:12
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