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]