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.

Sign up for GitHub

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

Jump to bottom

feat/display-subflow-content #2007

Merged
merged 4 commits into from
Sep 14, 2023
Merged

feat/display-subflow-content #2007

merged 4 commits into from
Sep 14, 2023

Conversation

brian-mulier-p
Copy link
Member

closes #774

@brian-mulier-p brian-mulier-p self-assigned this Aug 31, 2023
@brian-mulier-p
Copy link
Member Author

Screencast.from.31-08-2023.21.09.11.webm

@brian-mulier-p brian-mulier-p force-pushed the feat/display-subflow-content branch 3 times, most recently from 12b0c79 to f92ab78 Compare September 1, 2023 10:01
@brian-mulier-p brian-mulier-p force-pushed the feat/display-subflow-content branch 2 times, most recently from fe99460 to 05e88f2 Compare September 4, 2023 08:52
loicmathieu
loicmathieu requested changes Sep 4, 2023
View reviewed changes
Copy link
Member

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that you had to modify a method signature inside the FlowableTask is annoying as this interface may have been used on custom plugins.
If there is no other way we can live with it but it would be better if we avoid updating such interface.

core/src/main/java/io/kestra/core/models/tasks/Task.java Outdated Show resolved Hide resolved
ui/src/components/logs/LogList.vue Show resolved Hide resolved
core/src/main/java/io/kestra/core/services/AdvancedGraphService.java Outdated Show resolved Hide resolved
@brian-mulier-p brian-mulier-p force-pushed the feat/display-subflow-content branch from 05e88f2 to 79b8905 Compare September 4, 2023 16:53
@brian-mulier-p
Copy link
Member Author

For the flowable part maybe I can duplicate it without the subflow part to ensure backport compat with a default for the parent subflow one but I think people will need to agree and implement this part if they want proper subflow visualization so idk

@brian-mulier-p brian-mulier-p force-pushed the feat/display-subflow-content branch 2 times, most recently from 6df1d9f to 3a26593 Compare September 5, 2023 12:17
@anna-geller anna-geller mentioned this pull request Sep 5, 2023
Closed
@tchiotludo
Copy link
Member

@Skraye brian will refactor a lot this one, the current implementation break the immutability principle of Kestra and is to intrusive on the Flowable task.

@brian-mulier-p brian-mulier-p force-pushed the feat/display-subflow-content branch 5 times, most recently from 5d3d891 to 916168c Compare September 8, 2023 14:47
@tchiotludo tchiotludo force-pushed the feat/display-subflow-content branch from 916168c to 11ba4cb Compare September 8, 2023 22:22
@brian-mulier-p brian-mulier-p force-pushed the feat/display-subflow-content branch 2 times, most recently from 9f4857e to 0afea62 Compare September 12, 2023 10:42
@brian-mulier-p brian-mulier-p force-pushed the feat/display-subflow-content branch from 0afea62 to 99149cb Compare September 12, 2023 15:41
Skraye
Skraye approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@Skraye Skraye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brian-mulier-p brian-mulier-p force-pushed the feat/display-subflow-content branch from 99149cb to 1dd24b1 Compare September 14, 2023 08:59
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug D 2 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 33 Code Smells

73.1% 73.1% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@brian-mulier-p brian-mulier-p merged commit 4529282 into develop Sep 14, 2023
@brian-mulier-p brian-mulier-p deleted the feat/display-subflow-content branch September 14, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Skraye Skraye Skraye approved these changes

@tchiotludo tchiotludo Awaiting requested review from tchiotludo

@loicmathieu loicmathieu Awaiting requested review from loicmathieu

Assignees

@brian-mulier-p brian-mulier-p

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhance UI representation of subflows from a flow topology
4 participants
@brian-mulier-p @tchiotludo @loicmathieu @Skraye