Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate task triggers are incorrectly reported as circular task dependencies #583

Open
mountaindude opened this issue Dec 12, 2024 · 4 comments · May be fixed by #571
Open

Duplicate task triggers are incorrectly reported as circular task dependencies #583

mountaindude opened this issue Dec 12, 2024 · 4 comments · May be fixed by #571
Labels
bug Something isn't working

Comments

@mountaindude
Copy link
Contributor

What version of Ctrl-Q are you using?

4.4.0

What version of Node.js are you using? Not applicable if you use the standalone version of Ctrl-Q.

What command did you use to start Ctrl-Q?

ctrl-q.exe qseow task-get ...

What operating system are you using?

Windows 11

What CPU architecture are you using?

What Qlik Sense versions are you using?

Describe the Bug

Scenario:

  • TaskB has an upstream TaskA.
  • TaskB has two triggers that trigger when TaskA completes successfully.
    • Both triggers are identical except their names.

Ctrl-Q will report the second of TaskB's triggers as a circular dependency:

├─ TaskA
│ ├─ TaskB
│ │ ├─ TaskC
│ │ ├─ TaskD
│ └─ ==> !!! Cyclic dependency detected from task "TaskA" to "TaskB"

Expected Behavior

Ctrl-Q should identify that TaskB has several identical triggers (except their names) and warn about this.
There should not be a warning about a circular load chain.

To Reproduce

No response

@mountaindude mountaindude added the bug Something isn't working label Dec 12, 2024
@mountaindude
Copy link
Contributor Author

mountaindude commented Dec 12, 2024

Seems this only occur if the top level task (TaskA) is scheduled.
In that case the log output is

warn: Cyclic dependency detected in task tree, from task "undefined" to "Reload task of Ctrl-Q task tree app A". Won't go deeper.

That undefined is not good, but the main problem is that it is NOT a circular task chain.

@mountaindude mountaindude linked a pull request Dec 15, 2024 that will close this issue
@mountaindude
Copy link
Contributor Author

Latest update improved things, but reports a destination task of "undefined" in some occasions:

2024-12-16T08:04:16.501Z warn: Cyclic dependency detected in task tree. Won't go deeper.
2024-12-16T08:04:16.502Z warn:    From task : some-task
2024-12-16T08:04:16.502Z warn:    To task   : undefined

@mountaindude mountaindude reopened this Dec 16, 2024
@mountaindude
Copy link
Contributor Author

Currently circular task chains are detected by storing the entire task object in a set, and then checking if the set already contains a task.

Better would be to just store the task ID in the set.
Would be cleaner and provide a proper, deterministic way of detecting circular task chains.

@mountaindude
Copy link
Contributor Author

There's a corner case bug:

If a task chain at some point branches off into two (or more) paths, one of the paths will be examined first wrt circular task chains. The tasks in that branch will be remembered by Ctrl-Q.
When then examining the second branch, Ctrl-Q does not reset the memory of already visited tasks, and incorrectly triggers a circular task chain warning.

A proper Depth-First Search (DFS) algorithm is needed.
Something like this:

Represent the Tasks:

  • Use a dictionary or a class to represent each task, where each task has an identifier and a list of downstream tasks.

Initialize Data Structures:

  • Create a set to keep track of visited nodes.
  • Create a set to keep track of the current path in the DFS (this helps in identifying cycles).

DFS Function with Cycle Detection:

  • Implement a recursive DFS function that traverses the tree.
  • For each task, check if it is already part of the current path. If it is, a cycle is detected.
  • Add the task to both the visited set and the current path set before exploring its downstream tasks.
  • After exploring all downstream tasks, remove the task from the current path set.

Detect Cycles:

  • If during the DFS traversal, you find that a task is already in the current path set, return True indicating a cycle exists.
  • If the entire tree is traversed without finding any cycles, return False.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant