[GitHub] [incubator-devlake] CamilleTeruel commented on a diff in pull request #5138: 5081 fix missing records azuredevops

["CamilleTeruel (via GitHub)" <gi...@apache.org>]

CamilleTeruel commented on code in PR #5138:
URL: https://github.com/apache/incubator-devlake/pull/5138#discussion_r1190892823


##########
backend/python/README.md:
##########
@@ -232,6 +232,25 @@ Do not forget `table=True`, otherwise no table will be created in the database.
 
 To facilitate or even eliminate extraction, your tool models should be close to the raw data you collect. Note that if you collect data from a JSON REST API that uses camelCased properties, you can still define snake_cased attributes in your model. The camelCased attributes aliases will be generated, so no special care is needed during extraction.
 
+#### Migration of tool models
+
+Tool models, connection, scope and transformation rule types are stored in the DevLake database.
+When you change the definition of one of those types, you need to migrate the database.
+You should implement the migration logic in the model class by defining a `migrate` class method. This method takes a sqlalchemy session as argument that you can use to
+execute SQL `ALTER TABLE` statements.
+
+```python
+class User(ToolModel, table=True):
+    id: str = Field(primary_key=True)
+    name: str
+    email: str
+    age: int
+
+    @classmethod
+    def migrate(cls, session):
+        session.execute(f"ALTER TABLE {cls.__tablename__} ADD COLUMN age INT")

Review Comment:
   > Hi @CamilleTeruel, some clarifying questions on how tool model migrations work:
   > 
   >     1. When are these migrations run? Are they triggered from the Golang side?
   
   Yes, after loading a remote plugin, go-side calls run-migration.
    
   >     2. Our Golang side keeps track of migrations by date so each time only new migrations are run. How do we avoid re-running migrations that have already been run? 
   
   Right now we don't. This is just minimal support for migration to support this change. We must add versioning to migration functions.
   
   > Would the `migrate` function throw an error if it's run on a later version of DevLake?
   
   A migrate function can indeed throw an error if it has already run once, so we should be careful with migration until we have versionning support. But the `Job.migrate` function in particular is fine as it executes idempotent SQL statements. 
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org