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

Pipeline Node should not need to be connected to every task #57

Open
AlexandreSajus opened this issue May 25, 2023 · 18 comments
Open

Pipeline Node should not need to be connected to every task #57

AlexandreSajus opened this issue May 25, 2023 · 18 comments
Labels
📈 Improvement Improvement of a feature. 🟩 Priority: Low Low priority and doesn't need to be rushed Studio: Diagrams

Comments

@AlexandreSajus
Copy link

Description
We must connect the Pipeline Node to every Task Node we want to execute.
This was misleading during beta testing: testers would forget to connect new tasks, and since the error logs don't explain much, they could not find what was wrong.

A tester expected that the pipeline should only be connected to the first Task Node, every Task Node connected indirectly to this first Task Node should also be executed like here:
image

@AlexandreSajus AlexandreSajus added 🟩 Priority: Low Low priority and doesn't need to be rushed 📈 Improvement Improvement of a feature. labels May 25, 2023
@FabienLelaquais FabienLelaquais transferred this issue from Avaiga/taipy-core May 25, 2023
@jrobinAV
Copy link
Member

jrobinAV commented Jun 23, 2023

I believe there is a misunderstanding of what is a Pipeline vs a Scenario. In Taipy 3.0 it should be simpler.

Basically, a pipeline is a subset of tasks that can be executed together independently from the other scenario tasks.
When building your ScenarioConfig in studio, you want to define the sets of taskConfigs that belongs to the various PipelineConfigs created. There could be some taskConfig attacched to multiple PipelineConfigs and some TaskConfigs attached to zero pipelineConfig.

The proposal "the pipeline should only be connected to the first Task Node" does not make sense in this context.

Still the issue is interesting cause it shows that it is not intuitive. Probably both the Pipeline concept itself and Studio config are not intuitive regarding the usage of Pipeline.
I would recommend at this stage, to simply remove the Pipeline Box and the Scenario Box.

What do you think ?
@AlexandreSajus @FredLL-Avaiga @FlorianJacta @FabienLelaquais

@AlexandreSajus
Copy link
Author

I agree. Removing the Pipeline Box and Scenario Box would make things more straightforward and more intuitive

@FredLL-Avaiga
Copy link
Member

Shall we close this @AlexandreSajus @jrobinAV ?

@jrobinAV
Copy link
Member

I don't know.
A question has been raised. Do we want to remove the Pipeline box and the Scenario Box? Does it make sense for everyone?

@FredLL-Avaiga
Copy link
Member

FredLL-Avaiga commented Jul 19, 2023 via email

@AlexandreSajus
Copy link
Author

I don't know. A question has been raised. Do we want to remove the Pipeline box and the Scenario Box? Does it make sense for everyone?

In my mind, removing both of these would significantly improve accessibility which is the main bottleneck of Core currently.

@jrobinAV
Copy link
Member

@FredLL-Avaiga Yes, removing the pipeline would be part of 3.0 anyway.

In 3.0 ScenarioConfig will directly hold TaskConfigs and some additional data node configs unrelated to any taskConfig.

If we keep the Scenario Box, that means it will be linked to all the task boxes and all the additional data node boxes. I guess it will degrade the accessibility a lot.

@FabienLelaquais
Copy link
Member

If we use the link representation, yes.
But we can change that, and add a 'Holding Scenario' label in the task and invent something for building the relationship.
Now I suspect that tasks can be used by different scenario configs...

@jrobinAV
Copy link
Member

jrobinAV commented Jul 19, 2023

Yes, both task configs and data node configs may be used by multiple Scenario configs...

Maybe the Scenario config can be displayed as a "title" of the graph displayed. I don't know.
The difficulty would be on a global view with multiple scenario configs.

@FlorianJacta
Copy link
Member

My first instinct was to create rectangles around scenarios like in the documentation but it has its limits. Or we could represent scenarios with different colors.

Maybe the global view should be not the main view and the user should be "forced/guided" to go to the scenario view (the default option).

@jrobinAV
Copy link
Member

jrobinAV commented Dec 1, 2023

Up:
I believe it's still relevant to remove the Scenario box from the graph.
ARe we aligned on that?

@jrobinAV jrobinAV added good first issue New-contributor friendly and removed good first issue New-contributor friendly labels Dec 1, 2023
@FredLL-Avaiga
Copy link
Member

This issue is not relevant anymore
Scenario node is still shown when the whole configuration is displayer
It is not shown when the Scenario perspective is displayed

@FlorianJacta
Copy link
Member

It is still relevant because you have to connect your scenario config to the tasks, right?

@AlexandreSajus
Copy link
Author

Yeah, now the issue would be: either remove the scenario node (one toml file per scenario configuration) or do not make me connect the scenario node to each task

@FredLL-Avaiga
Copy link
Member

The scenario node is not visible in the Scenario Perspective and every Task that is added to the perspective is added to the scenario

@AlexandreSajus
Copy link
Author

Ah, I did not know that we did not have to connect tasks to the scenario node. Then it is fine. Maybe we should remove the scenario node then; what is its point?

@FlorianJacta
Copy link
Member

I also think so! When creating a configuration we could ask for a scenario name directly and not propose to view the whole configuration by default.

Also, when I add data nodes to my page, it goes to additional_datanodes which is correct but they should disappear from this section if I attach them to a task. Is it an issue? I don't think this leads to any error however.

image

@FredLL-Avaiga
Copy link
Member

task / additional_datanodes @jrobinAV do you confirm this is an issue ?
If so, @FlorianJacta can you create another issue ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📈 Improvement Improvement of a feature. 🟩 Priority: Low Low priority and doesn't need to be rushed Studio: Diagrams
Projects
None yet
Development

No branches or pull requests

5 participants