You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@devlake.apache.org by "mappjzc (via GitHub)" <gi...@apache.org> on 2023/02/08 08:36:57 UTC

[GitHub] [incubator-devlake] mappjzc opened a new pull request, #4359: Surpport Gitlab v11 surpport

mappjzc opened a new pull request, #4359:
URL: https://github.com/apache/incubator-devlake/pull/4359

   ### Summary
   Since GitLab needs to support the V11
   
   Therefore, the connection generation method has been modified. The new GitLab connection will detect whether the current connection needs to use the primary-key mode
   
   At the same time, ` account 'is added_ detail` `pipeline_ detail` `mr_ Detail 'and other data collection and conversion to deal with data missing in v11.
   
   ### Does this close any open issues?
   
   Closes #4136 
   
   ### Screenshots
   ![image](https://user-images.githubusercontent.com/2921251/217477552-52ea559d-dcdf-42c8-baca-78d60201a4af.png)
   
   ![image](https://user-images.githubusercontent.com/2921251/217474976-542d64d7-e944-47dd-b165-7fcb27acf206.png)
   


-- 
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] likyh commented on a diff in pull request #4359: Surpport Gitlab v11 surpport

Posted by "likyh (via GitHub)" <gi...@apache.org>.

likyh commented on code in PR #4359:
URL: https://github.com/apache/incubator-devlake/pull/4359#discussion_r1102217341


##########
backend/plugins/gitlab/tasks/mr_extractor.go:
##########
@@ -80,23 +82,36 @@ func ExtractApiMergeRequests(taskCtx plugin.SubTaskContext) errors.Error {
 	var labelTypeRegex *regexp.Regexp
 	var labelComponentRegex *regexp.Regexp
 	var prType = config.PrType
-	var err error
+
+	var err1 error
 	if len(prType) > 0 {
-		labelTypeRegex, err = regexp.Compile(prType)
-		if err != nil {
-			return errors.Default.Wrap(err, "regexp Compile prType failed")
+		labelTypeRegex, err1 = regexp.Compile(prType)
+		if err1 != nil {
+			return errors.Default.Wrap(err1, "regexp Compile prType failed")
 		}
 	}
 	var prComponent = config.PrComponent
 	if len(prComponent) > 0 {
-		labelComponentRegex, err = regexp.Compile(prComponent)
-		if err != nil {
-			return errors.Default.Wrap(err, "regexp Compile prComponent failed")
+		labelComponentRegex, err1 = regexp.Compile(prComponent)
+		if err1 != nil {
+			return errors.Default.Wrap(err1, "regexp Compile prComponent failed")
 		}
 	}
+	needDetail := false
+
 	extractor, err := api.NewApiExtractor(api.ApiExtractorArgs{
 		RawDataSubTaskArgs: *rawDataSubTaskArgs,
 		Extract: func(row *api.RawData) ([]interface{}, errors.Error) {

Review Comment:
   typo: row-> raw



-- 
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 #4359: Surpport Gitlab v11 surpport

Posted by "klesh (via GitHub)" <gi...@apache.org>.

klesh commented on code in PR #4359:
URL: https://github.com/apache/incubator-devlake/pull/4359#discussion_r1101443056


##########
backend/helpers/pluginhelper/api/api_client.go:
##########
@@ -178,6 +187,26 @@ func (apiClient *ApiClient) GetTimeout() time.Duration {
 	return apiClient.client.Timeout
 }
 
+// SetData FIXME ...
+func (apiClient *ApiClient) SetData(name string, data interface{}) {
+	apiClient.data_mutex.Lock()
+

Review Comment:
   Why empty lines?



##########
backend/helpers/pluginhelper/api/api_client.go:
##########
@@ -178,6 +187,26 @@ func (apiClient *ApiClient) GetTimeout() time.Duration {
 	return apiClient.client.Timeout
 }
 
+// SetData FIXME ...

Review Comment:
   Please fill the doc



##########
backend/plugins/gitlab/models/connection.go:
##########
@@ -18,19 +18,96 @@ limitations under the License.
 package models
 
 import (
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"fmt"
+	"net/http"
+
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api/apihelperabstract"
 )
 
 // GitlabConn holds the essential information to connect to the Gitlab API
 type GitlabConn struct {
-	helper.RestConnection `mapstructure:",squash"`
-	helper.AccessToken    `mapstructure:",squash"`
+	api.RestConnection `mapstructure:",squash"`
+	api.AccessToken    `mapstructure:",squash"`
+}
+
+const GitlabApiClientData_UserId string = "UserId"
+const GitlabApiClientData_ApiVersion string = "ApiVersion"
+
+// this function is used to rewrite the same function of AccessToken
+func (conn *GitlabConn) SetupAuthentication(request *http.Request) errors.Error {

Review Comment:
   No needed, please remove this function



##########
backend/plugins/gitlab/tasks/mr_extractor.go:
##########
@@ -155,7 +171,24 @@ func ExtractApiMergeRequests(taskCtx plugin.SubTaskContext) errors.Error {
 		return errors.Convert(err)
 	}
 
-	return extractor.Execute()
+	err = extractor.Execute()
+	if err != nil {
+		return err
+	}
+
+	if needDetail {

Review Comment:
   No, don't collect data during extraction.
   Please define another set of subtasks and check if they need to be actually executed inside subtasks



##########
backend/plugins/gitlab/tasks/pipeline_extractor.go:
##########
@@ -112,5 +117,23 @@ func ExtractApiPipelines(taskCtx plugin.SubTaskContext) errors.Error {
 		return err
 	}
 
-	return extractor.Execute()
+	err = extractor.Execute()
+	if err != nil {
+		return err
+	}
+
+	// If lost the detail info it means we have in the old version gitlab
+	// we should collect and Extract the detail for it
+	if needDetail {
+		err = CollectApiPipelineDetails(taskCtx)

Review Comment:
   Same as above



##########
backend/plugins/gitlab/models/connection.go:
##########
@@ -49,3 +131,5 @@ type ApiUserResponse struct {
 func (GitlabConnection) TableName() string {
 	return "_tool_gitlab_connections"
 }
+
+type AeAppKey api.AppKey

Review Comment:
   Why? I believe this is to be deleted



##########
backend/helpers/pluginhelper/api/api_client.go:
##########
@@ -349,6 +378,20 @@ func UnmarshalResponse(res *http.Response, v interface{}) errors.Error {
 	return nil
 }
 
+// UnmarshalResponseXML FIXME ...

Review Comment:
   Doc



-- 
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] likyh commented on a diff in pull request #4359: Surpport Gitlab v11 surpport

Posted by "likyh (via GitHub)" <gi...@apache.org>.

likyh commented on code in PR #4359:
URL: https://github.com/apache/incubator-devlake/pull/4359#discussion_r1102774217


##########
backend/plugins/gitlab/tasks/account_detail_collector.go:
##########
@@ -0,0 +1,117 @@
+/*
+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 tasks
+
+import (
+	"net/url"
+	"reflect"
+
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/plugins/gitlab/models"
+	"golang.org/x/mod/semver"
+)
+
+const RAW_USER_DETAIL_TABLE = "gitlab_api_user_details"
+
+var CollectAccountDetailsMeta = plugin.SubTaskMeta{
+	Name:             "collectAccountDetails",
+	EntryPoint:       CollectAccountDetails,
+	EnabledByDefault: true,
+	Description:      "collect gitlab user details",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CROSS},
+}
+
+func CollectAccountDetails(taskCtx plugin.SubTaskContext) errors.Error {
+
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_USER_DETAIL_TABLE)
+	logger := taskCtx.GetLogger()
+	logger.Info("collect gitlab user details")

Review Comment:
   delete debug code



##########
backend/plugins/gitlab/tasks/mr_detail_collector.go:
##########
@@ -0,0 +1,89 @@
+/*
+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 tasks
+
+import (
+	"reflect"
+
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+)
+
+const RAW_MERGE_REQUEST_DETAIL_TABLE = "gitlab_api_merge_request_details"
+
+var CollectApiMergeRequestDetailsMeta = plugin.SubTaskMeta{
+	Name:             "collectApiMergeRequestDetails",
+	EntryPoint:       CollectApiMergeRequestDetails,
+	EnabledByDefault: true,
+	Description:      "Collect merge request Details data from gitlab api",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CODE_REVIEW},
+}
+
+func CollectApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error {
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE)
+	collectorWithState, err := helper.NewApiCollectorWithState(*rawDataSubTaskArgs, data.CreatedDateAfter)
+	if err != nil {
+		return err
+	}
+
+	iterator, err := GetMergeRequestDetailsIterator(taskCtx, collectorWithState)
+	if err != nil {
+		return err
+	}
+
+	err = collectorWithState.InitCollector(helper.ApiCollectorArgs{
+		ApiClient:      data.ApiClient,
+		PageSize:       100,
+		Incremental:    false,
+		Input:          iterator,
+		UrlTemplate:    "projects/{{ .Params.ProjectId }}/merge_requests/{{ .Input.Iid }}",
+		Query:          GetQuery,

Review Comment:
   why this collector can run with pagination?



##########
backend/plugins/gitlab/tasks/mr_extractor.go:
##########
@@ -155,7 +170,12 @@ func ExtractApiMergeRequests(taskCtx plugin.SubTaskContext) errors.Error {
 		return errors.Convert(err)
 	}
 
-	return extractor.Execute()
+	err = extractor.Execute()

Review Comment:
   why expand this err?



##########
backend/plugins/gitlab/tasks/mr_extractor.go:
##########
@@ -107,6 +113,15 @@ func ExtractApiMergeRequests(taskCtx plugin.SubTaskContext) errors.Error {
 			if err != nil {
 				return nil, err
 			}
+
+			// if we can not find merged_at and closed_at info in the detail
+			// we need get detail for gitlab v11
+			if !strings.Contains(s, "\"merged_at\":") {
+				if !strings.Contains(s, "\"closed_at\":") {
+					gitlabMergeRequest.IsDetailRequired = true
+				}
+			}

Review Comment:
   I think this solution is a little confusing. Because all MRs have the same IsDetailRequired. 



##########
backend/plugins/gitlab/models/connection.go:
##########
@@ -18,19 +18,96 @@ limitations under the License.
 package models
 
 import (
-	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"fmt"
+	"net/http"
+
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api/apihelperabstract"
 )
 
 // GitlabConn holds the essential information to connect to the Gitlab API
 type GitlabConn struct {
-	helper.RestConnection `mapstructure:",squash"`
-	helper.AccessToken    `mapstructure:",squash"`
+	api.RestConnection `mapstructure:",squash"`
+	api.AccessToken    `mapstructure:",squash"`
+}
+
+const GitlabApiClientData_UserId string = "UserId"
+const GitlabApiClientData_ApiVersion string = "ApiVersion"
+
+// this function is used to rewrite the same function of AccessToken
+func (conn *GitlabConn) SetupAuthentication(request *http.Request) errors.Error {
+	return nil
+}
+
+// PrepareApiClient test api and set the IsPrivateToken,version,UserId and so on.
+func (conn *GitlabConn) PrepareApiClient(apiClient apihelperabstract.ApiClientAbstract) errors.Error {
+	header1 := http.Header{}
+	header1.Set("Authorization", fmt.Sprintf("Bearer %v", conn.Token))
+	// test request for access token
+	userResBody := &ApiUserResponse{}
+	res, err := apiClient.Get("user", nil, header1)
+	if res.StatusCode != http.StatusUnauthorized {
+		if err != nil {
+			return errors.Convert(err)
+		}
+		err = api.UnmarshalResponse(res, userResBody)
+		if err != nil {
+			return errors.Convert(err)
+		}
+
+		if res.StatusCode != http.StatusOK {
+			return errors.HttpStatus(res.StatusCode).New("unexpected status code while testing connection")
+		}
+		apiClient.SetHeaders(map[string]string{
+			"Authorization": fmt.Sprintf("Bearer %v", conn.Token),
+		})
+	} else {
+		header2 := http.Header{}
+		header2.Set("Private-Token", conn.Token)
+		res, err = apiClient.Get("user", nil, header2)
+		if err != nil {
+			return errors.Convert(err)
+		}
+		err = api.UnmarshalResponse(res, userResBody)
+		if err != nil {
+			return errors.Convert(err)
+		}
+
+		if res.StatusCode != http.StatusOK {
+			return errors.HttpStatus(res.StatusCode).New("unexpected status code while testing connection[PrivateToken]")
+		}
+		apiClient.SetHeaders(map[string]string{
+			"Private-Token": conn.Token,
+		})
+	}
+	// get gitlab version
+	versionResBody := &ApiVersionResponse{}
+	res, err = apiClient.Get("version", nil, nil)
+	if err != nil {
+		return errors.Convert(err)
+	}
+
+	err = api.UnmarshalResponse(res, versionResBody)
+	if err != nil {
+		return errors.Convert(err)
+	}
+
+	// add v for semver compare
+	if versionResBody.Version[0] != 'v' {
+		versionResBody.Version = "v" + versionResBody.Version
+	}
+
+	apiClient.SetData(GitlabApiClientData_UserId, userResBody.Id)

Review Comment:
   Maybe we can set these data into GitlabTaskData? Because it's not apiClient's data but plugin's data. 



##########
backend/plugins/gitlab/tasks/account_detail_collector.go:
##########
@@ -0,0 +1,117 @@
+/*
+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 tasks
+
+import (
+	"net/url"
+	"reflect"
+
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/plugins/gitlab/models"
+	"golang.org/x/mod/semver"
+)
+
+const RAW_USER_DETAIL_TABLE = "gitlab_api_user_details"
+
+var CollectAccountDetailsMeta = plugin.SubTaskMeta{
+	Name:             "collectAccountDetails",
+	EntryPoint:       CollectAccountDetails,
+	EnabledByDefault: true,
+	Description:      "collect gitlab user details",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CROSS},
+}
+
+func CollectAccountDetails(taskCtx plugin.SubTaskContext) errors.Error {
+
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_USER_DETAIL_TABLE)
+	logger := taskCtx.GetLogger()
+	logger.Info("collect gitlab user details")
+
+	if !NeedAccountDetails(data.ApiClient) {
+		logger.Info("Don't need collect gitlab user details,skip")
+		return nil
+	}
+
+	iterator, err := GetAccountsIterator(taskCtx)
+	if err != nil {
+		return err
+	}
+	defer iterator.Close()
+
+	collector, err := api.NewApiCollector(api.ApiCollectorArgs{
+		RawDataSubTaskArgs: *rawDataSubTaskArgs,
+		ApiClient:          data.ApiClient,
+		UrlTemplate:        "/projects/{{ .Params.ProjectId }}/members/{{ .Input.GitlabId }}",
+		Input:              iterator,
+		//PageSize:           100,
+		Query: func(reqData *api.RequestData) (url.Values, errors.Error) {
+			query := url.Values{}
+			// query.Set("sort", "asc")

Review Comment:
   delete unused code



##########
backend/plugins/gitlab/tasks/account_extractor.go:
##########
@@ -65,5 +67,10 @@ func ExtractAccounts(taskCtx plugin.SubTaskContext) errors.Error {
 		return err
 	}
 
-	return extractor.Execute()
+	err = extractor.Execute()

Review Comment:
   why do you expand this?



##########
backend/plugins/gitlab/tasks/mr_detail_collector.go:
##########
@@ -0,0 +1,89 @@
+/*
+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 tasks
+
+import (
+	"reflect"
+
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+)
+
+const RAW_MERGE_REQUEST_DETAIL_TABLE = "gitlab_api_merge_request_details"
+
+var CollectApiMergeRequestDetailsMeta = plugin.SubTaskMeta{
+	Name:             "collectApiMergeRequestDetails",
+	EntryPoint:       CollectApiMergeRequestDetails,
+	EnabledByDefault: true,
+	Description:      "Collect merge request Details data from gitlab api",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CODE_REVIEW},
+}
+
+func CollectApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error {
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE)
+	collectorWithState, err := helper.NewApiCollectorWithState(*rawDataSubTaskArgs, data.CreatedDateAfter)
+	if err != nil {
+		return err
+	}
+
+	iterator, err := GetMergeRequestDetailsIterator(taskCtx, collectorWithState)
+	if err != nil {
+		return err
+	}
+
+	err = collectorWithState.InitCollector(helper.ApiCollectorArgs{
+		ApiClient:      data.ApiClient,
+		PageSize:       100,
+		Incremental:    false,
+		Input:          iterator,
+		UrlTemplate:    "projects/{{ .Params.ProjectId }}/merge_requests/{{ .Input.Iid }}",
+		Query:          GetQuery,
+		GetTotalPages:  GetTotalPagesFromResponse,

Review Comment:
   delete page size if not used



##########
backend/plugins/gitlab/tasks/mr_detail_collector.go:
##########
@@ -0,0 +1,89 @@
+/*
+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 tasks
+
+import (
+	"reflect"
+
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+)
+
+const RAW_MERGE_REQUEST_DETAIL_TABLE = "gitlab_api_merge_request_details"
+
+var CollectApiMergeRequestDetailsMeta = plugin.SubTaskMeta{
+	Name:             "collectApiMergeRequestDetails",
+	EntryPoint:       CollectApiMergeRequestDetails,
+	EnabledByDefault: true,
+	Description:      "Collect merge request Details data from gitlab api",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CODE_REVIEW},
+}
+
+func CollectApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error {
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE)
+	collectorWithState, err := helper.NewApiCollectorWithState(*rawDataSubTaskArgs, data.CreatedDateAfter)
+	if err != nil {
+		return err
+	}
+
+	iterator, err := GetMergeRequestDetailsIterator(taskCtx, collectorWithState)
+	if err != nil {
+		return err
+	}
+
+	err = collectorWithState.InitCollector(helper.ApiCollectorArgs{
+		ApiClient:      data.ApiClient,
+		PageSize:       100,

Review Comment:
   delete page size if not used



##########
backend/plugins/gitlab/tasks/mr_detail_extractor.go:
##########
@@ -0,0 +1,121 @@
+/*
+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 tasks
+
+import (
+	"encoding/json"
+	"regexp"
+
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/plugins/gitlab/models"
+)
+
+var ExtractApiMergeRequestDetailsMeta = plugin.SubTaskMeta{
+	Name:             "extractApiMergeRequestDetails",
+	EntryPoint:       ExtractApiMergeRequestDetails,
+	EnabledByDefault: true,
+	Description:      "Extract raw merge request Details data into tool layer table GitlabMergeRequest and GitlabReviewer",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CODE_REVIEW},
+}
+
+func ExtractApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error {
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE)
+	config := data.Options.GitlabTransformationRule
+	var labelTypeRegex *regexp.Regexp
+	var labelComponentRegex *regexp.Regexp
+	var prType = config.PrType
+	var err error
+	if len(prType) > 0 {
+		labelTypeRegex, err = regexp.Compile(prType)
+		if err != nil {
+			return errors.Default.Wrap(err, "regexp Compile prType failed")
+		}
+	}
+	var prComponent = config.PrComponent
+	if len(prComponent) > 0 {
+		labelComponentRegex, err = regexp.Compile(prComponent)
+		if err != nil {
+			return errors.Default.Wrap(err, "regexp Compile prComponent failed")
+		}
+	}
+	extractor, err := api.NewApiExtractor(api.ApiExtractorArgs{
+		RawDataSubTaskArgs: *rawDataSubTaskArgs,
+		Extract: func(row *api.RawData) ([]interface{}, errors.Error) {
+			mr := &MergeRequestRes{}
+			err := errors.Convert(json.Unmarshal(row.Data, mr))
+			if err != nil {
+				return nil, err
+			}
+
+			gitlabMergeRequest, err := convertMergeRequest(mr)
+			if err != nil {
+				return nil, err
+			}
+			results := make([]interface{}, 0, len(mr.Reviewers)+1)
+			gitlabMergeRequest.ConnectionId = data.Options.ConnectionId
+			gitlabMergeRequest.IsDetailRequired = true

Review Comment:
   why set `IsDetailRequired` here? I think it only should set on mr extracter.



##########
backend/plugins/gitlab/tasks/pipeline_detail_collector.go:
##########
@@ -0,0 +1,98 @@
+/*
+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 tasks
+
+import (
+	"net/url"
+	"reflect"
+
+	"github.com/apache/incubator-devlake/core/dal"
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+)
+
+const RAW_PIPELINE_DETAILS_TABLE = "gitlab_api_pipeline_details"
+
+var CollectApiPipelineDetailsMeta = plugin.SubTaskMeta{
+	Name:             "collectApiPipelineDetails",
+	EntryPoint:       CollectApiPipelineDetails,
+	EnabledByDefault: true,
+	Description:      "Collect pipeline details data from gitlab api",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CICD},
+}
+
+func CollectApiPipelineDetails(taskCtx plugin.SubTaskContext) errors.Error {
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_PIPELINE_DETAILS_TABLE)
+	collectorWithState, err := helper.NewApiCollectorWithState(*rawDataSubTaskArgs, data.CreatedDateAfter)
+	if err != nil {
+		return err
+	}
+
+	incremental := collectorWithState.IsIncremental()

Review Comment:
   why do you add incremental but query all data without filter by latestUpdated.



##########
backend/plugins/gitlab/tasks/pipeline_extractor.go:
##########
@@ -112,5 +116,10 @@ func ExtractApiPipelines(taskCtx plugin.SubTaskContext) errors.Error {
 		return err
 	}
 
-	return extractor.Execute()
+	err = extractor.Execute()

Review Comment:
   as above



-- 
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 #4359: Surpport Gitlab v11 surpport

Posted by "klesh (via GitHub)" <gi...@apache.org>.

klesh commented on code in PR #4359:
URL: https://github.com/apache/incubator-devlake/pull/4359#discussion_r1102783890


##########
backend/plugins/gitlab/tasks/account_extractor.go:
##########
@@ -65,5 +67,10 @@ func ExtractAccounts(taskCtx plugin.SubTaskContext) errors.Error {
 		return err
 	}
 
-	return extractor.Execute()
+	err = extractor.Execute()

Review Comment:
   Why?



##########
backend/plugins/gitlab/tasks/mr_detail_extractor.go:
##########
@@ -0,0 +1,121 @@
+/*
+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 tasks
+
+import (
+	"encoding/json"
+	"regexp"
+
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/plugins/gitlab/models"
+)
+
+var ExtractApiMergeRequestDetailsMeta = plugin.SubTaskMeta{
+	Name:             "extractApiMergeRequestDetails",
+	EntryPoint:       ExtractApiMergeRequestDetails,
+	EnabledByDefault: true,
+	Description:      "Extract raw merge request Details data into tool layer table GitlabMergeRequest and GitlabReviewer",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CODE_REVIEW},
+}
+
+func ExtractApiMergeRequestDetails(taskCtx plugin.SubTaskContext) errors.Error {
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_MERGE_REQUEST_DETAIL_TABLE)
+	config := data.Options.GitlabTransformationRule
+	var labelTypeRegex *regexp.Regexp
+	var labelComponentRegex *regexp.Regexp
+	var prType = config.PrType
+	var err error
+	if len(prType) > 0 {
+		labelTypeRegex, err = regexp.Compile(prType)
+		if err != nil {
+			return errors.Default.Wrap(err, "regexp Compile prType failed")
+		}
+	}
+	var prComponent = config.PrComponent
+	if len(prComponent) > 0 {
+		labelComponentRegex, err = regexp.Compile(prComponent)
+		if err != nil {
+			return errors.Default.Wrap(err, "regexp Compile prComponent failed")
+		}
+	}
+	extractor, err := api.NewApiExtractor(api.ApiExtractorArgs{
+		RawDataSubTaskArgs: *rawDataSubTaskArgs,
+		Extract: func(row *api.RawData) ([]interface{}, errors.Error) {
+			mr := &MergeRequestRes{}
+			err := errors.Convert(json.Unmarshal(row.Data, mr))
+			if err != nil {
+				return nil, err
+			}
+
+			gitlabMergeRequest, err := convertMergeRequest(mr)
+			if err != nil {
+				return nil, err
+			}
+			results := make([]interface{}, 0, len(mr.Reviewers)+1)
+			gitlabMergeRequest.ConnectionId = data.Options.ConnectionId
+			gitlabMergeRequest.IsDetailRequired = true

Review Comment:
   This should be `false` since we can be sure that detail HAS BEEN COLLECTED at this point, right?



##########
backend/plugins/gitlab/tasks/pipeline_detail_extractor.go:
##########
@@ -0,0 +1,90 @@
+/*
+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 tasks
+
+import (
+	"encoding/json"
+
+	"github.com/apache/incubator-devlake/core/errors"
+	"github.com/apache/incubator-devlake/core/plugin"
+	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+	"github.com/apache/incubator-devlake/plugins/gitlab/models"
+)
+
+var ExtractApiPipelineDetailsMeta = plugin.SubTaskMeta{
+	Name:             "extractApiPipelineDetails",
+	EntryPoint:       ExtractApiPipelineDetails,
+	EnabledByDefault: true,
+	Description:      "Extract raw pipeline details data into tool layer table GitlabPipeline",
+	DomainTypes:      []string{plugin.DOMAIN_TYPE_CICD},
+}
+
+func ExtractApiPipelineDetails(taskCtx plugin.SubTaskContext) errors.Error {
+	rawDataSubTaskArgs, data := CreateRawDataSubTaskArgs(taskCtx, RAW_PIPELINE_DETAILS_TABLE)
+
+	extractor, err := api.NewApiExtractor(api.ApiExtractorArgs{
+		RawDataSubTaskArgs: *rawDataSubTaskArgs,
+		Extract: func(row *api.RawData) ([]interface{}, errors.Error) {
+			// create gitlab commit
+			gitlabApiPipeline := &ApiPipeline{}
+			err := errors.Convert(json.Unmarshal(row.Data, gitlabApiPipeline))
+			if err != nil {
+				return nil, err
+			}
+
+			if gitlabApiPipeline.UpdatedAt != nil && gitlabApiPipeline.CreatedAt != nil {
+				gitlabApiPipeline.Duration = int(gitlabApiPipeline.UpdatedAt.ToTime().Sub(gitlabApiPipeline.CreatedAt.ToTime()).Seconds())
+			}
+			gitlabPipeline := &models.GitlabPipeline{
+				GitlabId:         gitlabApiPipeline.Id,
+				ProjectId:        data.Options.ProjectId,
+				WebUrl:           gitlabApiPipeline.WebUrl,
+				Status:           gitlabApiPipeline.Status,
+				GitlabCreatedAt:  api.Iso8601TimeToTime(gitlabApiPipeline.CreatedAt),
+				GitlabUpdatedAt:  api.Iso8601TimeToTime(gitlabApiPipeline.UpdatedAt),
+				StartedAt:        api.Iso8601TimeToTime(gitlabApiPipeline.StartedAt),
+				FinishedAt:       api.Iso8601TimeToTime(gitlabApiPipeline.FinishedAt),
+				Duration:         gitlabApiPipeline.Duration,
+				ConnectionId:     data.Options.ConnectionId,
+				IsDetailRequired: true,

Review Comment:
   Same as above



##########
backend/plugins/gitlab/tasks/mr_extractor.go:
##########
@@ -155,7 +170,12 @@ func ExtractApiMergeRequests(taskCtx plugin.SubTaskContext) errors.Error {
 		return errors.Convert(err)
 	}
 
-	return extractor.Execute()

Review Comment:
   why...



##########
backend/plugins/gitlab/tasks/pipeline_extractor.go:
##########
@@ -112,5 +116,10 @@ func ExtractApiPipelines(taskCtx plugin.SubTaskContext) errors.Error {
 		return err
 	}
 
-	return extractor.Execute()

Review Comment:
   Why?



-- 
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