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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to