You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by "ech018 (via GitHub)" <gi...@apache.org> on 2023/05/16 10:01:54 UTC
[GitHub] [incubator-devlake] ech018 opened a new pull request, #5208: Remove workflow name length validation
ech018 opened a new pull request, #5208:
URL: https://github.com/apache/incubator-devlake/pull/5208
### ⚠️ Pre Checklist
- [ ] I have added relevant tests.
- [ ] I will add labels to the PR, such as `pr-type/bug-fix`, `pr-type/feature-development`, etc.
### Summary
Github workflows may have longer than 255 characters long names. In order to sync it I changed gorm type from `varchar(255)` to `text` for `GithubRun` type `Name` field.
### Other Information
Error received while syncing github repository:
```
time="2023-05-09 08:37:03" level=error msg=" [pipeline service] [pipeline #3] [task #25] subtask extractRuns ended unexpectedly
Wraps: (2) error adding result to batch
Wraps: (3) Error 1406: Data too long for column 'name' at row 17
Wraps: (4) Error 1406: Data too long for column 'name' at row 17
Error types: (1) *hintdetail.withDetail (2) *hintdetail.withDetail (3) *hintdetail.withDetail (4) *mysql.MySQLError"
```
--
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 #5208: Use text instead of varchar(255) for GitHubRun's name
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #5208:
URL: https://github.com/apache/incubator-devlake/pull/5208#discussion_r1198455933
##########
backend/plugins/github/models/migrationscripts/20230518_fix_run_name_to_text.go:
##########
@@ -0,0 +1,69 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package migrationscripts
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/helpers/migrationhelper"
+)
+
+type fixRunNameToText struct{}
+
+type githubRun20230518_old struct {
+ ConnectionId uint64 `gorm:"primaryKey"`
+ RepoId int `gorm:"primaryKey"`
+
+ Name string `gorm:"type:varchar(255)"`
+}
+type githubRun20230518 struct {
+ ConnectionId uint64 `gorm:"primaryKey"`
+ RepoId int `gorm:"primaryKey"`
+
+ Name string `gorm:"type:text"`
+}
+
+func (*fixRunNameToText) Up(baseRes context.BasicRes) errors.Error {
+ err := migrationhelper.TransformColumns(
+ baseRes,
+ &fixRunNameToText{},
+ "_tool_github_runs",
+ []string{"name"},
+ func(src *githubRun20230518_old) (*githubRun20230518, errors.Error) {
+ return &githubRun20230518{
+ ConnectionId: src.ConnectionId,
+ RepoId: src.RepoId,
+ Name: src.Name,
+ }, nil
+ },
+ )
+
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
+func (*fixRunNameToText) Version() uint64 {
+ return 20230518000001
Review Comment:
It is date+time, timestamp is ok too.
--
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] ech018 commented on a diff in pull request #5208: Remove workflow name length validation
Posted by "ech018 (via GitHub)" <gi...@apache.org>.
ech018 commented on code in PR #5208:
URL: https://github.com/apache/incubator-devlake/pull/5208#discussion_r1197687621
##########
backend/plugins/github/models/migrationscripts/20230518_fix_run_name_to_text.go:
##########
@@ -0,0 +1,69 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package migrationscripts
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/helpers/migrationhelper"
+)
+
+type fixRunNameToText struct{}
+
+type githubRun20230518_old struct {
+ ConnectionId uint64 `gorm:"primaryKey"`
+ RepoId int `gorm:"primaryKey"`
+
+ Name string `gorm:"type:varchar(255)"`
+}
+type githubRun20230518 struct {
+ ConnectionId uint64 `gorm:"primaryKey"`
+ RepoId int `gorm:"primaryKey"`
+
+ Name string `gorm:"type:text"`
+}
+
+func (*fixRunNameToText) Up(baseRes context.BasicRes) errors.Error {
+ err := migrationhelper.TransformColumns(
+ baseRes,
+ &fixRunNameToText{},
+ "_tool_github_runs",
Review Comment:
Not sure about this string, I assume it should be same as in [run.go](https://github.com/apache/incubator-devlake/blob/243cc8a80aa5b37828e2a142ac9f7e3269b7e1dc/backend/plugins/github/models/run.go#L59), please fix it if I'm wrong.
--
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 pull request #5208: Remove workflow name length validation
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on PR #5208:
URL: https://github.com/apache/incubator-devlake/pull/5208#issuecomment-1552575118
Thanks for your contribution. However, changing the `type` only would not be sufficient.
In order for the existing table and records to be updated, we have to provide a database migration script like [this one](https://github.com/apache/incubator-devlake/blob/243cc8a80aa5b37828e2a142ac9f7e3269b7e1dc/backend/plugins/gitlab/models/migrationscripts/20220906_fix_duration_to_float8.go) and add it to the [register.go](https://github.com/apache/incubator-devlake/blob/348ba7a09c9b99cc61984c9f7c2589d6fde1c0bf/backend/plugins/github/models/migrationscripts/register.go#L39) so it can be picked and executed during boot-up of the `devlake`
--
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] ech018 commented on pull request #5208: Remove workflow name length validation
Posted by "ech018 (via GitHub)" <gi...@apache.org>.
ech018 commented on PR #5208:
URL: https://github.com/apache/incubator-devlake/pull/5208#issuecomment-1552895878
@klesh Added migration script, please review carefully as this is my first contribution. Thank you.
--
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 #5208: Use text instead of varchar(255) for GitHubRun's name
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #5208:
URL: https://github.com/apache/incubator-devlake/pull/5208#discussion_r1198456120
##########
backend/plugins/github/models/migrationscripts/20230518_fix_run_name_to_text.go:
##########
@@ -0,0 +1,69 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package migrationscripts
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/helpers/migrationhelper"
+)
+
+type fixRunNameToText struct{}
+
+type githubRun20230518_old struct {
+ ConnectionId uint64 `gorm:"primaryKey"`
+ RepoId int `gorm:"primaryKey"`
+
+ Name string `gorm:"type:varchar(255)"`
+}
+type githubRun20230518 struct {
+ ConnectionId uint64 `gorm:"primaryKey"`
+ RepoId int `gorm:"primaryKey"`
+
+ Name string `gorm:"type:text"`
+}
+
+func (*fixRunNameToText) Up(baseRes context.BasicRes) errors.Error {
+ err := migrationhelper.TransformColumns(
+ baseRes,
+ &fixRunNameToText{},
+ "_tool_github_runs",
Review Comment:
Yes, you are totally correct.
--
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 #5208: Use text instead of varchar(255) for GitHubRun's name
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh merged PR #5208:
URL: https://github.com/apache/incubator-devlake/pull/5208
--
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] ech018 commented on a diff in pull request #5208: Remove workflow name length validation
Posted by "ech018 (via GitHub)" <gi...@apache.org>.
ech018 commented on code in PR #5208:
URL: https://github.com/apache/incubator-devlake/pull/5208#discussion_r1197686067
##########
backend/plugins/github/models/migrationscripts/20230518_fix_run_name_to_text.go:
##########
@@ -0,0 +1,69 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package migrationscripts
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/helpers/migrationhelper"
+)
+
+type fixRunNameToText struct{}
+
+type githubRun20230518_old struct {
+ ConnectionId uint64 `gorm:"primaryKey"`
+ RepoId int `gorm:"primaryKey"`
+
+ Name string `gorm:"type:varchar(255)"`
+}
+type githubRun20230518 struct {
+ ConnectionId uint64 `gorm:"primaryKey"`
+ RepoId int `gorm:"primaryKey"`
+
+ Name string `gorm:"type:text"`
+}
+
+func (*fixRunNameToText) Up(baseRes context.BasicRes) errors.Error {
+ err := migrationhelper.TransformColumns(
+ baseRes,
+ &fixRunNameToText{},
+ "_tool_github_runs",
+ []string{"name"},
+ func(src *githubRun20230518_old) (*githubRun20230518, errors.Error) {
+ return &githubRun20230518{
+ ConnectionId: src.ConnectionId,
+ RepoId: src.RepoId,
+ Name: src.Name,
+ }, nil
+ },
+ )
+
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
+func (*fixRunNameToText) Version() uint64 {
+ return 20230518000001
Review Comment:
I was not sure what convention do you use to set this number.
I figured it is a date + timestamp, but not sure which timestamp is correct.
--
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