You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by li...@apache.org on 2023/02/22 14:38:08 UTC

[incubator-devlake] branch main updated: refactor: github to adopt timeAfter param (#4485)

This is an automated email from the ASF dual-hosted git repository.

likyh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-devlake.git


The following commit(s) were added to refs/heads/main by this push:
     new d9a0f7f51 refactor: github to adopt timeAfter param (#4485)
d9a0f7f51 is described below

commit d9a0f7f512651ed84029f8bd8743d7365386d6c5
Author: Klesh Wong <zh...@merico.dev>
AuthorDate: Wed Feb 22 22:38:03 2023 +0800

    refactor: github to adopt timeAfter param (#4485)
    
    * refactor: github to adopt timeAfter param
    
    * fix: issue/commit collector supports timeFilter
---
 backend/plugins/github/tasks/comment_collector.go           |  3 +--
 backend/plugins/github/tasks/commit_collector.go            |  5 ++++-
 backend/plugins/github/tasks/issue_collector.go             |  6 ++++--
 backend/plugins/github/tasks/pr_commit_collector.go         | 11 ++++++-----
 backend/plugins/github/tasks/pr_review_collector.go         |  8 ++++----
 backend/plugins/github/tasks/pr_review_comment_collector.go |  1 -
 6 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/backend/plugins/github/tasks/comment_collector.go b/backend/plugins/github/tasks/comment_collector.go
index 35dd29042..f57c733a1 100644
--- a/backend/plugins/github/tasks/comment_collector.go
+++ b/backend/plugins/github/tasks/comment_collector.go
@@ -32,7 +32,7 @@ const RAW_COMMENTS_TABLE = "github_api_comments"
 
 func CollectApiComments(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*GithubTaskData)
-	collectorWithState, err := helper.NewApiCollectorWithState(helper.RawDataSubTaskArgs{
+	collectorWithState, err := helper.NewStatefulApiCollector(helper.RawDataSubTaskArgs{
 		Ctx: taskCtx,
 		Params: GithubApiParams{
 			ConnectionId: data.Options.ConnectionId,
@@ -54,7 +54,6 @@ func CollectApiComments(taskCtx plugin.SubTaskContext) errors.Error {
 		Query: func(reqData *helper.RequestData) (url.Values, errors.Error) {
 			query := url.Values{}
 			query.Set("state", "all")
-			// if data.CreatedDateAfter != nil, we set since once
 			if data.TimeAfter != nil {
 				// Note that `since` is for filtering records by the `updated` time
 				// which is not ideal for semantic reasons and would result in slightly more records than expected.
diff --git a/backend/plugins/github/tasks/commit_collector.go b/backend/plugins/github/tasks/commit_collector.go
index 17be5f014..3136a4c98 100644
--- a/backend/plugins/github/tasks/commit_collector.go
+++ b/backend/plugins/github/tasks/commit_collector.go
@@ -40,7 +40,7 @@ var CollectApiCommitsMeta = plugin.SubTaskMeta{
 
 func CollectApiCommits(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*GithubTaskData)
-	collectorWithState, err := helper.NewApiCollectorWithState(helper.RawDataSubTaskArgs{
+	collectorWithState, err := helper.NewStatefulApiCollector(helper.RawDataSubTaskArgs{
 		Ctx: taskCtx,
 		Params: GithubApiParams{
 			ConnectionId: data.Options.ConnectionId,
@@ -73,6 +73,9 @@ func CollectApiCommits(taskCtx plugin.SubTaskContext) errors.Error {
 		Query: func(reqData *helper.RequestData) (url.Values, errors.Error) {
 			query := url.Values{}
 			query.Set("state", "all")
+			if data.TimeAfter != nil {
+				query.Set("since", data.TimeAfter.String())
+			}
 			if incremental {
 				query.Set("since", collectorWithState.LatestState.LatestSuccessStart.String())
 			}
diff --git a/backend/plugins/github/tasks/issue_collector.go b/backend/plugins/github/tasks/issue_collector.go
index d70b11967..89e07fc2d 100644
--- a/backend/plugins/github/tasks/issue_collector.go
+++ b/backend/plugins/github/tasks/issue_collector.go
@@ -40,7 +40,7 @@ var CollectApiIssuesMeta = plugin.SubTaskMeta{
 
 func CollectApiIssues(taskCtx plugin.SubTaskContext) errors.Error {
 	data := taskCtx.GetData().(*GithubTaskData)
-	collectorWithState, err := helper.NewApiCollectorWithState(helper.RawDataSubTaskArgs{
+	collectorWithState, err := helper.NewStatefulApiCollector(helper.RawDataSubTaskArgs{
 		Ctx: taskCtx,
 		Params: GithubApiParams{
 			ConnectionId: data.Options.ConnectionId,
@@ -73,7 +73,9 @@ func CollectApiIssues(taskCtx plugin.SubTaskContext) errors.Error {
 		Query: func(reqData *helper.RequestData) (url.Values, errors.Error) {
 			query := url.Values{}
 			query.Set("state", "all")
-			// data.CreatedDateAfter need to be used to filter data, but now no params supported
+			if data.TimeAfter != nil {
+				query.Set("since", data.TimeAfter.String())
+			}
 			if incremental {
 				query.Set("since", collectorWithState.LatestState.LatestSuccessStart.String())
 			}
diff --git a/backend/plugins/github/tasks/pr_commit_collector.go b/backend/plugins/github/tasks/pr_commit_collector.go
index 29da846fb..0c876936a 100644
--- a/backend/plugins/github/tasks/pr_commit_collector.go
+++ b/backend/plugins/github/tasks/pr_commit_collector.go
@@ -55,7 +55,7 @@ func CollectApiPullRequestCommits(taskCtx plugin.SubTaskContext) errors.Error {
 	db := taskCtx.GetDal()
 	data := taskCtx.GetData().(*GithubTaskData)
 
-	collectorWithState, err := helper.NewApiCollectorWithState(helper.RawDataSubTaskArgs{
+	collectorWithState, err := helper.NewStatefulApiCollector(helper.RawDataSubTaskArgs{
 		Ctx: taskCtx,
 		Params: GithubApiParams{
 			ConnectionId: data.Options.ConnectionId,
@@ -74,11 +74,12 @@ func CollectApiPullRequestCommits(taskCtx plugin.SubTaskContext) errors.Error {
 		dal.From(models.GithubPullRequest{}.TableName()),
 		dal.Where("repo_id = ? and connection_id=?", data.Options.GithubId, data.Options.ConnectionId),
 	}
-	if collectorWithState.CreatedDateAfter != nil {
-		clauses = append(clauses, dal.Where("github_created_at > ?", *collectorWithState.CreatedDateAfter))
-	}
+	// incremental collection, no need to care about the timeFilter since it has to be collected by PR
 	if incremental {
-		clauses = append(clauses, dal.Where("github_updated_at > ?", *collectorWithState.LatestState.LatestSuccessStart))
+		clauses = append(
+			clauses,
+			dal.Where("github_updated_at > ?", collectorWithState.LatestState.LatestSuccessStart),
+		)
 	}
 	cursor, err := db.Cursor(
 		clauses...,
diff --git a/backend/plugins/github/tasks/pr_review_collector.go b/backend/plugins/github/tasks/pr_review_collector.go
index 958ab4dfb..86d022930 100644
--- a/backend/plugins/github/tasks/pr_review_collector.go
+++ b/backend/plugins/github/tasks/pr_review_collector.go
@@ -65,11 +65,11 @@ func CollectApiPullRequestReviews(taskCtx plugin.SubTaskContext) errors.Error {
 		dal.From(models.GithubPullRequest{}.TableName()),
 		dal.Where("repo_id = ? and connection_id=?", data.Options.GithubId, data.Options.ConnectionId),
 	}
-	if collectorWithState.CreatedDateAfter != nil {
-		clauses = append(clauses, dal.Where("github_created_at > ?", *collectorWithState.CreatedDateAfter))
-	}
 	if incremental {
-		clauses = append(clauses, dal.Where("github_updated_at > ?", *collectorWithState.LatestState.LatestSuccessStart))
+		clauses = append(
+			clauses,
+			dal.Where("github_updated_at > ?", collectorWithState.LatestState.LatestSuccessStart),
+		)
 	}
 	cursor, err := db.Cursor(
 		clauses...,
diff --git a/backend/plugins/github/tasks/pr_review_comment_collector.go b/backend/plugins/github/tasks/pr_review_comment_collector.go
index c8c02869c..fb69becfb 100644
--- a/backend/plugins/github/tasks/pr_review_comment_collector.go
+++ b/backend/plugins/github/tasks/pr_review_comment_collector.go
@@ -56,7 +56,6 @@ func CollectPrReviewComments(taskCtx plugin.SubTaskContext) errors.Error {
 		UrlTemplate: "repos/{{ .Params.Name }}/pulls/comments",
 		Query: func(reqData *helper.RequestData) (url.Values, errors.Error) {
 			query := url.Values{}
-			// if data.CreatedDateAfter != nil, we set since once
 			if data.TimeAfter != nil {
 				// Note that `since` is for filtering records by the `updated` time
 				// which is not ideal for semantic reasons and would result in slightly more records than expected.