You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by GitBox <gi...@apache.org> on 2022/10/18 11:37:54 UTC
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #3471: refactor: implement the interface core.MigrationScript
klesh commented on code in PR #3471:
URL: https://github.com/apache/incubator-devlake/pull/3471#discussion_r998087319
##########
plugins/jira/models/migrationscripts/20220407_add_source_table.go:
##########
@@ -18,28 +18,22 @@ limitations under the License.
package migrationscripts
import (
- "context"
"github.com/apache/incubator-devlake/errors"
+ "github.com/apache/incubator-devlake/helpers/migrationhelper"
+ "github.com/apache/incubator-devlake/plugins/core"
"github.com/apache/incubator-devlake/plugins/jira/models/migrationscripts/archived"
- "gorm.io/gorm"
)
-type InitSchemas struct{}
+type addSourceTable struct{}
-func (*InitSchemas) Up(ctx context.Context, db *gorm.DB) errors.Error {
- m := db.Migrator()
- if m.HasTable(&archived.JiraConnection{}) {
- return nil
- }
- return errors.Convert(db.Migrator().AutoMigrate(
- &archived.JiraSource{},
- ))
+func (*addSourceTable) Up(basicRes core.BasicRes) errors.Error {
+ return errors.Convert(migrationhelper.AutoMigrateTables(basicRes, &archived.JiraSource{}))
Review Comment:
`errors.Convert` is not needed.
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -18,30 +18,29 @@ limitations under the License.
package migrationscripts
import (
- "context"
"encoding/base64"
"strings"
"time"
"github.com/apache/incubator-devlake/config"
"github.com/apache/incubator-devlake/errors"
+ "github.com/apache/incubator-devlake/helpers/migrationhelper"
"github.com/apache/incubator-devlake/plugins/core"
"github.com/apache/incubator-devlake/plugins/jira/models/migrationscripts/archived"
- "gorm.io/gorm"
)
-type JiraConnectionNew struct {
+type jiraConnectionNew struct {
Review Comment:
model name should be suffixed with the `Version`
##########
plugins/jira/models/migrationscripts/20220505_rename_source_table.go:
##########
@@ -18,60 +18,21 @@ limitations under the License.
package migrationscripts
import (
- "context"
-
"github.com/apache/incubator-devlake/errors"
- "github.com/apache/incubator-devlake/models/migrationscripts/archived"
- "gorm.io/gorm"
+ "github.com/apache/incubator-devlake/plugins/core"
)
-type JiraConnectionV010 struct {
- archived.Model
- Name string `gorm:"type:varchar(100);uniqueIndex" json:"name" validate:"required"`
- Endpoint string `json:"endpoint" validate:"required"`
- BasicAuthEncoded string `json:"basicAuthEncoded" validate:"required"`
- EpicKeyField string `gorm:"type:varchar(50);" json:"epicKeyField"`
- StoryPointField string `gorm:"type:varchar(50);" json:"storyPointField"`
- RemotelinkCommitShaPattern string `gorm:"type:varchar(255);comment='golang regexp, the first group will be recognized as commit sha, ref https://github.com/google/re2/wiki/Syntax'" json:"remotelinkCommitShaPattern"`
- Proxy string `json:"proxy"`
- RateLimit int `comment:"api request rate limt per hour" json:"rateLimit"`
-}
-
-func (JiraConnectionV010) TableName() string {
- return "_tool_jira_connections"
-}
-
-type JiraSource struct {
- archived.Model
- Name string `gorm:"type:varchar(100);uniqueIndex" json:"name" validate:"required"`
- Endpoint string `json:"endpoint" validate:"required"`
- BasicAuthEncoded string `json:"basicAuthEncoded" validate:"required"`
- EpicKeyField string `gorm:"type:varchar(50);" json:"epicKeyField"`
- StoryPointField string `gorm:"type:varchar(50);" json:"storyPointField"`
- RemotelinkCommitShaPattern string `gorm:"type:varchar(255);comment='golang regexp, the first group will be recognized as commit sha, ref https://github.com/google/re2/wiki/Syntax'" json:"remotelinkCommitShaPattern"`
- Proxy string `json:"proxy"`
- RateLimit int `comment:"api request rate limt per second"`
-}
-
-func (JiraSource) TableName() string {
- return "_tool_jira_sources"
-}
-
type renameSourceTable struct{}
-func (*renameSourceTable) Up(ctx context.Context, db *gorm.DB) errors.Error {
- err := db.Migrator().RenameTable(JiraSource{}, JiraConnectionV010{})
+func (*renameSourceTable) Up(basicRes core.BasicRes) errors.Error {
+ err := basicRes.GetDal().RenameTable("_tool_jira_sources", "_tool_jira_connections")
return errors.Convert(err)
Review Comment:
Same as above
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -90,46 +89,38 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) errors.Error {
&archived.JiraSprintIssue{},
&archived.JiraWorklog{},
); err != nil {
- return errors.Convert(err)
+ return err
}
-
- // In order to avoid postgres reporting "cached plan must not change result type",
- // we have to avoid loading the _tool_jira_connections table before our migration. The trick is to rename it before loading.
- // so need to use JiraConnectionOld, JiraConnectionNew and JiraConnection to operate.
- // step1: create _tool_jira_connections_new table
- // step2: rename _tool_jira_connections to _tool_jira_connections_old
- // step3: select data from _tool_jira_connections_old and insert date to _tool_jira_connections_new
- // step4: rename _tool_jira_connections_new to _tool_jira_connections
+ dal := basicRes.GetDal()
// step1
- if err = db.Migrator().CreateTable(&JiraConnectionNew{}); err != nil {
- return errors.Convert(err)
+ if err = migrationhelper.AutoMigrateTables(basicRes, &jiraConnectionNew{}); err != nil {
+ return err
}
//nolint:errcheck
- defer db.Migrator().DropTable(&JiraConnectionNew{})
+ defer dal.DropTables(&jiraConnectionNew{})
// step2
- if err = db.Migrator().RenameTable("_tool_jira_connections", "_tool_jira_connections_old"); err != nil {
- return errors.Convert(err)
+ if err = dal.RenameTable("_tool_jira_connections", "_tool_jira_connections_old"); err != nil {
+ return err
}
defer func() {
if err != nil {
- err = db.Migrator().RenameTable("_tool_jira_connections_old", "_tool_jira_connections")
+ err = dal.RenameTable("_tool_jira_connections_old", "_tool_jira_connections")
} else {
- err = db.Migrator().DropTable(&JiraConnectionV011Old{})
+ err = dal.DropTables(&jiraConnectionV011Old{})
}
}()
// step3
- var result *gorm.DB
- var jiraConns []JiraConnectionV011Old
- result = db.Find(&jiraConns)
- if result.Error != nil {
- return errors.Convert(result.Error)
+ var jiraConns []jiraConnectionV011Old
+ err = dal.All(&jiraConns)
+ if err != nil {
+ return err
}
for _, v := range jiraConns {
Review Comment:
Please use `migrationhelper.TransformTable` instead
--
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