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/09/05 03:16:54 UTC
[GitHub] [incubator-devlake] abeizn opened a new pull request, #2950: Fix#2935
abeizn opened a new pull request, #2950:
URL: https://github.com/apache/incubator-devlake/pull/2950
# Summary
old: rename a->a
new: rename a->b->a
### Does this close any open issues?
Closes #2935
### Screenshots
Include any relevant screenshots here.
### 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 #2950: fix: cached plan must not change result type
Posted by GitBox <gi...@apache.org>.
klesh commented on code in PR #2950:
URL: https://github.com/apache/incubator-devlake/pull/2950#discussion_r962600222
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -99,8 +117,15 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) error {
}
defer db.Migrator().DropTable(&JiraConnectionTemp{})
+ // cached plan must not change result type in postgres, so need to update db name
+ err = db.Migrator().RenameTable(JiraConnectionV011{}, JiraConnectionV011Bak{})
Review Comment:
Seems like we could pass the table name as `string` directly, will it be simpler?
--
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 #2950: fix: cached plan must not change result type
Posted by GitBox <gi...@apache.org>.
klesh commented on code in PR #2950:
URL: https://github.com/apache/incubator-devlake/pull/2950#discussion_r962762482
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -92,61 +94,79 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) error {
return err
}
- // get connection history data
- err = db.Migrator().CreateTable(&JiraConnectionTemp{})
+ // due to cached plan must not change result type in postgres,
+ // so need to use JiraConnectionOld, JiraConnectionNew and JiraConnection to operate.
Review Comment:
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.
--
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] abeizn commented on a diff in pull request #2950: fix: cached plan must not change result type
Posted by GitBox <gi...@apache.org>.
abeizn commented on code in PR #2950:
URL: https://github.com/apache/incubator-devlake/pull/2950#discussion_r962631310
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -99,8 +117,15 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) error {
}
defer db.Migrator().DropTable(&JiraConnectionTemp{})
+ // cached plan must not change result type in postgres, so need to update db name
+ err = db.Migrator().RenameTable(JiraConnectionV011{}, JiraConnectionV011Bak{})
Review Comment:
> Seems like we could pass the table name as `string` directly, will it be simpler? 
updated
--
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] warren830 merged pull request #2950: fix: cached plan must not change result type
Posted by GitBox <gi...@apache.org>.
warren830 merged PR #2950:
URL: https://github.com/apache/incubator-devlake/pull/2950
--
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] warren830 commented on a diff in pull request #2950: Fix#2935
Posted by GitBox <gi...@apache.org>.
warren830 commented on code in PR #2950:
URL: https://github.com/apache/incubator-devlake/pull/2950#discussion_r962554205
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -99,8 +117,15 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) error {
}
defer db.Migrator().DropTable(&JiraConnectionTemp{})
+ // cached plan must not change result type in postgres, so need to update db name
+ err = db.Migrator().RenameTable(JiraConnectionV011{}, JiraConnectionV011Bak{})
Review Comment:
if you just want to change table name, I think it'd better delete all fields in `JiraConnectionV011`
--
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] warren830 commented on a diff in pull request #2950: fix: cached plan must not change result type
Posted by GitBox <gi...@apache.org>.
warren830 commented on code in PR #2950:
URL: https://github.com/apache/incubator-devlake/pull/2950#discussion_r962776199
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -92,61 +94,79 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) error {
return err
}
- // get connection history data
- err = db.Migrator().CreateTable(&JiraConnectionTemp{})
+ // due to cached plan must not change result type in postgres,
+ // 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
+ err = db.Migrator().CreateTable(&JiraConnectionNew{})
if err != nil {
return err
}
- defer db.Migrator().DropTable(&JiraConnectionTemp{})
+ defer db.Migrator().DropTable(&JiraConnectionNew{})
+ // step2
+ err = db.Migrator().RenameTable(JiraConnectionV011, JiraConnectionV011Old{})
+ if err != nil {
+ return err
+ }
+ defer func() {
+ if err != nil {
+ err = db.Migrator().RenameTable(JiraConnectionV011Old{}, JiraConnectionV011)
+ } else {
+ err = db.Migrator().DropTable(&JiraConnectionV011Old{})
+ }
+ }()
+
+ // step3
var result *gorm.DB
- var jiraConns []JiraConnectionV011
+ var jiraConns []JiraConnectionV011Old
result = db.Find(&jiraConns)
+ if result.Error != nil {
+ return result.Error
+ }
- if result.Error == nil {
- for _, v := range jiraConns {
- conn := &JiraConnectionTemp{}
- 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", errors.AsUserMessage())
- }
- auth, err := core.Decrypt(encKey, v.BasicAuthEncoded)
- if err != nil {
- return err
- }
- pk, err := base64.StdEncoding.DecodeString(auth)
+ 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", errors.AsUserMessage())
+ }
+ auth, err := core.Decrypt(encKey, v.BasicAuthEncoded)
+ if err != nil {
+ return err
+ }
+ pk, err := base64.StdEncoding.DecodeString(auth)
+ if err != nil {
+ return 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 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 err
- }
- // create
- tx := db.Create(&conn)
- if tx.Error != nil {
- return errors.Default.Wrap(tx.Error, "error adding connection to DB")
- }
+ // create
+ tx := db.Create(&conn)
+ if tx.Error != nil {
+ return errors.Default.Wrap(tx.Error, "error adding connection to DB")
}
}
}
- err = db.Migrator().DropTable(&JiraConnectionV011{})
- if err != nil {
- return err
- }
-
- err = db.Migrator().RenameTable(JiraConnectionTemp{}, archived.JiraConnection{})
+ // step4
+ err = db.Migrator().RenameTable(JiraConnectionNew{}, archived.JiraConnection{})
Review Comment:
I think we can directly use table name instead of archived.JiraConnection{}
Two reasons:
1. more intuitive
2. gorm can do one less parse
--
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] warren830 commented on a diff in pull request #2950: Fix#2935
Posted by GitBox <gi...@apache.org>.
warren830 commented on code in PR #2950:
URL: https://github.com/apache/incubator-devlake/pull/2950#discussion_r962560011
##########
plugins/jira/models/migrationscripts/20220716_add_init_tables.go:
##########
@@ -99,8 +117,15 @@ func (*addInitTables) Up(ctx context.Context, db *gorm.DB) error {
}
defer db.Migrator().DropTable(&JiraConnectionTemp{})
+ // cached plan must not change result type in postgres, so need to update db name
+ err = db.Migrator().RenameTable(JiraConnectionV011{}, JiraConnectionV011Bak{})
+ if err != nil {
+ return err
+ }
+ defer db.Migrator().DropTable(&JiraConnectionV011Bak{})
+
var result *gorm.DB
- var jiraConns []JiraConnectionV011
+ var jiraConns []JiraConnectionV011Bak
result = db.Find(&jiraConns)
if result.Error == nil {
Review Comment:
Also, please do not use if like this, you can simply use `if result.Error != nil` and return err. Code like this make it difficult to understand
--
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