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:25:12 UTC
[GitHub] [incubator-devlake] mindlesscloud opened a new pull request, #3471: refactor: implement the interface core.MigrationScript
mindlesscloud opened a new pull request, #3471:
URL: https://github.com/apache/incubator-devlake/pull/3471
# Summary
Refactor all migration scripts to implement the interface core.MigrationScript.
Related to #3176 Migration adopts BasicRes
### Does this close any open issues?
Closes #3469
### Screenshots
records in `_devlake_migration_history`:
created tables:
### Other Information
Any other information that is important to this PR.
--
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
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #3471: refactor: implement the interface core.MigrationScript
Posted by GitBox <gi...@apache.org>.
klesh commented on code in PR #3471:
URL: https://github.com/apache/incubator-devlake/pull/3471#discussion_r999057282
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -90,94 +87,54 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) errors.Error {
&archived.JiraSprintIssue{},
&archived.JiraWorklog{},
); err != nil {
- return errors.Convert(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
-
- // step1
- if err = db.Migrator().CreateTable(&JiraConnectionNew{}); err != nil {
- return errors.Convert(err)
- }
- //nolint:errcheck
- defer db.Migrator().DropTable(&JiraConnectionNew{})
-
- // step2
- if err = db.Migrator().RenameTable("_tool_jira_connections", "_tool_jira_connections_old"); err != nil {
- return errors.Convert(err)
- }
- defer func() {
- if err != nil {
- err = db.Migrator().RenameTable("_tool_jira_connections_old", "_tool_jira_connections")
- } else {
- err = db.Migrator().DropTable(&JiraConnectionV011Old{})
- }
- }()
-
- // step3
- var result *gorm.DB
- var jiraConns []JiraConnectionV011Old
- result = db.Find(&jiraConns)
- if result.Error != nil {
- return errors.Convert(result.Error)
+ return err
}
-
- for _, v := range jiraConns {
- conn := &JiraConnectionNew{}
- conn.ID = v.ID
- conn.Name = v.Name
- conn.Endpoint = v.Endpoint
- conn.Proxy = v.Proxy
- conn.RateLimitPerHour = v.RateLimit
-
- c := config.GetConfig()
- encKey := c.GetString(core.EncodeKeyEnvStr)
- if encKey == "" {
- return errors.BadInput.New("jira v0.11 invalid encKey")
- }
- var auth string
- if auth, err = core.Decrypt(encKey, v.BasicAuthEncoded); err != nil {
- return errors.Convert(err)
- }
- var pk []byte
- pk, err = base64.StdEncoding.DecodeString(auth)
- if err != nil {
- return errors.Convert(err)
- }
- originInfo := strings.Split(string(pk), ":")
- if len(originInfo) == 2 {
- conn.Username = originInfo[0]
- conn.Password, err = core.Encrypt(encKey, originInfo[1])
- if err != nil {
- return errors.Convert(err)
+ err = migrationhelper.TransformTable(
+ basicRes,
+ script,
+ "_tool_jira_connections",
+ func(old *jiraConnectionV011Old) (*jiraConnection20220716, errors.Error) {
Review Comment:
Can we use `jiraConnection20220716Before` and `jiraConnection20220716After`, I think they are more descriptive. What do you think?
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -90,94 +87,54 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) errors.Error {
&archived.JiraSprintIssue{},
&archived.JiraWorklog{},
); err != nil {
- return errors.Convert(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
-
- // step1
- if err = db.Migrator().CreateTable(&JiraConnectionNew{}); err != nil {
- return errors.Convert(err)
- }
- //nolint:errcheck
- defer db.Migrator().DropTable(&JiraConnectionNew{})
-
- // step2
- if err = db.Migrator().RenameTable("_tool_jira_connections", "_tool_jira_connections_old"); err != nil {
- return errors.Convert(err)
- }
- defer func() {
- if err != nil {
- err = db.Migrator().RenameTable("_tool_jira_connections_old", "_tool_jira_connections")
- } else {
- err = db.Migrator().DropTable(&JiraConnectionV011Old{})
- }
- }()
-
- // step3
- var result *gorm.DB
- var jiraConns []JiraConnectionV011Old
- result = db.Find(&jiraConns)
- if result.Error != nil {
- return errors.Convert(result.Error)
+ return err
}
-
- for _, v := range jiraConns {
- conn := &JiraConnectionNew{}
- conn.ID = v.ID
- conn.Name = v.Name
- conn.Endpoint = v.Endpoint
- conn.Proxy = v.Proxy
- conn.RateLimitPerHour = v.RateLimit
-
- c := config.GetConfig()
- encKey := c.GetString(core.EncodeKeyEnvStr)
- if encKey == "" {
- return errors.BadInput.New("jira v0.11 invalid encKey")
- }
- var auth string
- if auth, err = core.Decrypt(encKey, v.BasicAuthEncoded); err != nil {
- return errors.Convert(err)
- }
- var pk []byte
- pk, err = base64.StdEncoding.DecodeString(auth)
- if err != nil {
- return errors.Convert(err)
- }
- originInfo := strings.Split(string(pk), ":")
- if len(originInfo) == 2 {
- conn.Username = originInfo[0]
- conn.Password, err = core.Encrypt(encKey, originInfo[1])
- if err != nil {
- return errors.Convert(err)
+ err = migrationhelper.TransformTable(
+ basicRes,
+ script,
+ "_tool_jira_connections",
+ func(old *jiraConnectionV011Old) (*jiraConnection20220716, errors.Error) {
+ conn := &jiraConnection20220716{}
+ conn.ID = old.ID
+ conn.Name = old.Name
+ conn.Endpoint = old.Endpoint
+ conn.Proxy = old.Proxy
+ conn.RateLimitPerHour = old.RateLimit
+
+ c := config.GetConfig()
+ encKey := c.GetString(core.EncodeKeyEnvStr)
Review Comment:
Please move 104~107 to the outer function so we check it only once.
--
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
[GitHub] [incubator-devlake] klesh merged pull request #3471: refactor: implement the interface core.MigrationScript
Posted by GitBox <gi...@apache.org>.
klesh merged PR #3471:
URL: https://github.com/apache/incubator-devlake/pull/3471
--
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
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #3471: refactor: implement the interface core.MigrationScript
Posted by GitBox <gi...@apache.org>.
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