refactor: add subtask register for jira #5725
Conversation
chenggui53
changed the title
chenggui53
changed the title
chenggui53
changed the title
chenggui53
force-pushed
the
add-subtask-register-for-jira
branch
3 times, most recently
from
d54f920
to
5df4a3a
Compare
chenggui53
force-pushed
the
add-subtask-register-for-jira
branch
2 times, most recently
from
7c71b30
to
ab77882
Compare
chenggui53
force-pushed
the
add-subtask-register-for-jira
branch
from
ab77882
to
6e43334
Compare
|
Hi, @chenggui53 Thanks for the good work. I may have to postpone this PR since we are now testing the existing code for v0.19. |
@klesh Okay, I'll try to see if the subtaskmeta register part can be further optimized or other things i can do until v0.19 is done. |
There was a problem hiding this comment.
Hi, @chenggui53 We are ready to accept new PRs for v0.20 due to release-v0.19 is checked out.
I reviewed this one up to the epic_extractor part and I think we need to discuss the solution for the existing cyclic-dependency situation before we can proceed.
Sorry for the delay, 🥲
| @@ -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 | |||
There was a problem hiding this 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:
- Define a new table named
_tool_jira_pre_accounts - Other subtasks including
issue_extractorsandissue_changelog_extractorwould write to_tool_jira_pre_accountsinstead of_tool_jira_accounts - The
account_collectorwould read from_tool_jira_pre_accountsas well
By doing so, we break the cyclic dependency and simplify the flow.
There was a problem hiding this comment.
@klesh Thanks for the comments, I'll fix this type problem ASAP.
| 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}, |
There was a problem hiding this 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
| @@ -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 | |||
There was a problem hiding this 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?
|
Hi, @chenggui53 We are planning to code freeze next Tue 9-26, I would like to know if it works for you, do you think the PR can be updated in time? |
@klesh Sorry, there have been a lot of chores lately, which has resulted in no updates being submitted. Can you help move it to the next version, I'll be working on an update in the next few days, but there's no guarantee it'll be merged in on 9.26. |
|
@chenggui53 Thanks for the quick response. No problem, take your time. |
|
@chenggui53 There is a conflict, maybe you can fix it. |
|
This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs. |
|
This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request. |
Summary
What does this PR do?
like pr #4896 and #5411, add subtask register for jira
Does this close any open issues?
Closes #4413
Screenshots
Include any relevant screenshots here.