[GitHub] [incubator-devlake] klesh commented on a diff in pull request #5725: refactor: add subtask register for jira

["klesh (via GitHub)" <gi...@apache.org>]

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


##########
backend/plugins/jira/tasks/account_collector.go:
##########
@@ -38,6 +42,10 @@ var CollectAccountsMeta = plugin.SubTaskMeta{
 	EnabledByDefault: true,
 	Description:      "collect Jira accounts, does not support either timeFilter or diffSync.",
 	DomainTypes:      []string{plugin.DOMAIN_TYPE_CROSS},
+	DependencyTables: []string{
+		// models.JiraAccount{}.TableName(), // cursor, config not regard as dependency

Review Comment:
   If I understand it correctly, it is commented out because other subtasks would write records into this table (namely `_tool_jira_account` for `account_collector` to collect details one by one.
   However, the `account_extractor` would write to the same table which creates a cyclic dependency.
   
   I think the current logic is problematic, it would be better if we do the following:
   1. Define a new table named `_tool_jira_pre_accounts`
   2. Other subtasks including `issue_extractors` and `issue_changelog_extractor` would write to  `_tool_jira_pre_accounts` instead of  `_tool_jira_accounts`
   3. The `account_collector` would read from  `_tool_jira_pre_accounts` as well
   
   By doing so,  we break the cyclic dependency and simplify the flow.



##########
backend/plugins/jira/tasks/account_convertor.go:
##########
@@ -26,15 +28,22 @@ import (
 	"github.com/apache/incubator-devlake/core/plugin"
 	"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
 	"github.com/apache/incubator-devlake/plugins/jira/models"
-	"reflect"
 )
 
+func init() {
+	RegisterSubtaskMeta(&ConvertAccountsMeta)
+}
+
 var ConvertAccountsMeta = plugin.SubTaskMeta{
 	Name:             "convertAccounts",
 	EntryPoint:       ConvertAccounts,
 	EnabledByDefault: true,
 	Description:      "convert Jira accounts",
 	DomainTypes:      []string{plugin.DOMAIN_TYPE_CROSS},
+	DependencyTables: []string{
+		models.JiraAccount{}.TableName(), // cursor
+		RAW_USERS_TABLE},

Review Comment:
   I think `RAW_USERS_TABLE` is not needed by the converter according to https://github.com/apache/incubator-devlake/pull/5725/files#diff-1c7d0a017c0981e5b254d6ffa97ac7cff85da42d9883f3c27ebf7de1fc5c7016R56-R59



##########
backend/plugins/jira/tasks/epic_collector.go:
##########
@@ -44,6 +48,11 @@ var CollectEpicsMeta = plugin.SubTaskMeta{
 	EnabledByDefault: true,
 	Description:      "collect Jira epics from all boards, does not support either timeFilter or diffSync.",
 	DomainTypes:      []string{plugin.DOMAIN_TYPE_TICKET, plugin.DOMAIN_TYPE_CROSS},
+	DependencyTables: []string{
+		models.JiraIssue{}.TableName(), // cursor
+		// models.JiraBoardIssue{}.TableName()}, // cursor, board issue not regard as dependency cause epic extractor will produce board issue result

Review Comment:
   Oh... This is tricky... apparently, we have another cyclic dependency here. 🥲.
   Maybe we could create new tables `_jira_board_epics` and `_jira_epics` to avoid such a thing?



##########
backend/plugins/jira/tasks/epic_collector.go:
##########
@@ -44,6 +48,11 @@ var CollectEpicsMeta = plugin.SubTaskMeta{
 	EnabledByDefault: true,
 	Description:      "collect Jira epics from all boards, does not support either timeFilter or diffSync.",
 	DomainTypes:      []string{plugin.DOMAIN_TYPE_TICKET, plugin.DOMAIN_TYPE_CROSS},
+	DependencyTables: []string{
+		models.JiraIssue{}.TableName(), // cursor
+		// models.JiraBoardIssue{}.TableName()}, // cursor, board issue not regard as dependency cause epic extractor will produce board issue result

Review Comment:
   Oh... This is tricky... apparently, we have another cyclic dependency here. 🥲.
   Maybe we could create new tables `_jira_board_epics` and `_jira_epics` to avoid such a thing?



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