Display warning or notice on explicit dependency with `chain` mode DAG

[yottahmd]

Feedback from @ghansham:

Display a clear visual warning or notice on the Web UI when explicit dependencies are configured in a chain mode DAG, since this mode automatically links dependencies between steps.

[ghansham]

Or we can switch default to graph and explicit set to chain.

[yottahmd]

Yes, that is also a possible resolution. As of now, user can set “type: graph” in the base DAG to make it default across all DAGs.

[kriyanshii]

worked a version

[ghansham]

@yottahmd, in migrate step also we can set type: graph for all dags which have depends mentioned with proper log messages mentioning:

pre v1.17 style dag definition found: dag type not found in dag <dagname>, with depends defined, setting dag type to graph to get it working correctly in v1.17+ versions. See
https://docs.dagu.cloud/writing-workflows/examples#execution-mode-chain-vs-graph for details.

The default chain mode anyhow was not supported in pre 1.17 era. So hardly there could be dags with depends not defined except single step dags.

[yottahmd]

@ghansham I see, yeah that's possible. I'm wondering what if an user wants to mix depends setting and chain style, though.

What do you think about setting type: graph in the base config in your migration step?

[ghansham]

@yottahmd mixing them is pretty much possible. But I feel it needs lot of thinking to get it working. The reason is chain type is going to define dependencies based on the sequence in which steps are mentioned. While editing (adding/removing steps), if sequence changes unintentionally by user, dependency graph shall change. It may be quite cumbersome and confusing for the user. See #1262 for one such example. Even when we were migrating, we also faced similar issues. @kriyanshii fixed it by setting type: graph for every single dag. And we had lots and lots of dags (> 30000)

Settingtype in base.yaml in migration step makes a lot of sense to me. Then every dag which does not have type mentioned in dag definition should inherit this value. Otherwise, it shall be set to chain. The default value of type in base.yaml shall be chain if not specified explicitly. And the type value mentioned in dag definition shall override the type value mentioned in base.yaml. Right?

Still we should keep this warning in place to keep rarest conditions in check leading to confusion. This shall be helpful in debugging such cases.

[yottahmd]

@ghansham Yes, that’s correct.

Addingtype in base.yaml makes lot of sense. Then every dag which does not have type mentioned in dag definition should inherit this value. Otherwise, it shall be set to chain. The default value of type in base.yaml shall be chain if not specified explicitly. And the type value mentioned in dag definition shall override the type value mentioned in base.yaml. Right?

Sure, I agree that when depends is defined explicitly, we should require type: graph. To make the behavior clearer, perhaps we should make it an error instead of a warning. It might also be better to enhance the chain mode, so user can achieve the same thing without depends settings.

For instance, parallel steps without depends:

steps:
- command: echo "start"
  - command: echo "parallel 1"
  - command: echo "parallel 2"
- command: echo "finish"

[ghansham]

Agree. This could be a step towards dags with no depends entries. Do we have option to add step name, capture output, continueOn and pre conditions in this syntax?