Feature/explorer executor groups

[RealZimboGuy]

No description provided.

[RealZimboGuy]

[eputtone]

Thanks for the contribution!

I didn't have time to make a review in depth today, and the number of cross-cutting changes is large. Some of the general concerns that surface at first.

1. In most deployments, there's a single executor group. Hence, most often showing executor group "all over the Explorer" adds to the cognitive burden for most users ("should I understand what this is?")
2. Juggling with multiple ways of specifying allowed executor groups in nFlow engine and REST API adds complexity.
   • Example 1: if you have nFlow instance processing workflows in executor group X and with property nflow.db.query_all_executors=true, we have to be extra-careful that widened API executor group access to workflow management doesn't leak into the workflow processing side.
   • Example 2: workflow implementations often use nFlow Java API, and breaking the executor group borders there might be unexpected, when you actually tried enable it in Explorer only.

So I'd like to discuss an alternative solution ("management node"), that might be relatively easily reachable from your existing PR, and would (IMHO) simplify things.

1. If nflow.db.query_all_executors=true, then set executorGroupCondition to 1 = 1 here.
   • Now all query APIs should work over all executor groups, and you don't need a new API, service and DAO for querying executors over different groups. And e.g WorkflowDefinitionDao.queryStoredWorkflowDefinitions() can still keep the executorGroupCondition, which is anyways needed for the default execution group isolation.
   • Update APIs still need the new executor group parameters, but in the DAOs the logic should always be: if nflow.db.query_all_executors=true use the given executor groups directly, otherwise use the group from the properties.
2. If nflow.db.query_all_executors=true, then prevent the node from being started if dispatcher is autostarting (e.g. here).
   • By having a separate management node without workflow processing, all of type 2 concerns (above) disappear as nodes processing workflows have always a single executor group.
   • Btw, in this approach nflow.db.query_all_executors would be renamed to nflow.management_node (boolean).
3. Explorer changes are basically fine, but "Executor group" components could be hidden if all the executors (returned by the existing API) are all from the same group.
   • Solves type 1 concern (above)

[eputtone]

A lot of formatting changes, and formatting has indeed been inconsistent.

In addition to applying the formatting, it would be nice to have a script-definition e.g. in package.json to apply the formatting. So that it's consistent and not change depending every when the developer changes.

[RealZimboGuy]

interesting thought, thanks for digging into it a bit.

1- Agreed, might as well " hide it from basic implementations"

i do think your idea of a "management node" would be a good approach
this is the way i understand what you said

so a management node is started with a query all executors and the explorer flag for "advanced" or whatever its called. Users would then browse nflow explorer on that management node where dispatcher is disabled

[eputtone]

Yes, that diagram visualizes the idea accurately 👍

An additional deployment is needed for the management node - but I think it's a relatively small price to pay for keeping the roles straight.

Either the executor group isolation is effective (=nFlow executor processing workflows) or it is fully removed (=nFlow management node). Having both in the same nFlow node i.e. executor group isolation effective in workflow processing and lowered in the REST/Java API feels error-prone.

I have a feeling that your PR is not too far from implementing the idea.

[RealZimboGuy]

ill have another go through over the next week or so and see where i get to. (y) there is that other PR if you could have a look at, some small tweaks to the graph