Skip to content

Conversation

@harshit-soora
Copy link
Contributor

@harshit-soora harshit-soora commented Oct 11, 2025

Workflows Epic Link - #4338
UI Pull Request - #4604

  • Saved the edge and launcher position in backend and allow submitting workflow.
  • Added workflow submit, reset, save and back buttons.
  • Code tested for
    • submitting workflow
    • save and reload workflow

This is how new UI looks like:
image

@Bubballoo3
Copy link
Contributor

Could you add some system tests that take the workflow from start to finish through the gui? They will be essential to maintenance and consistency as this component is developed. I know getting Capybara to work the gui and checking the pixel positions of the display will be a bit of work, but that will be the starting point of all future tests on workflows so will be worth it in the long run.

@harshit-soora
Copy link
Contributor Author

Could you add some system tests

That is actually in plan, will discuss with Jeff on how can we have a end-to-end test as we will need scheduler to see if scripts executed in order expected.

@Bubballoo3
Copy link
Contributor

This seems pretty good, still need to write a few individual scripts to see if the submission and running piece is working as expected, but it looks like I can no longer delete edges. Could you see if this happens on your end as well?

@harshit-soora
Copy link
Contributor Author

harshit-soora commented Oct 18, 2025

no longer delete edges

It is working as expected, there is one thing that is needed on edge is a UI wrapper, so when the mouse pointer is close it gets highlighted or increase the select range. Right now, the mouse needs to be exactly on top of edge.

I have fixed this in UI pull request and rebased this branch with that, so should work seamlessly now.

@harshit-soora harshit-soora force-pushed the backend branch 2 times, most recently from a90a6dc to 27a2ff4 Compare October 18, 2025 03:25
Added feature to restore from backend metadata and check authenticity based upon when it was saved

Added zoom to metadata, tested rigorously
Copy link
Contributor

@Bubballoo3 Bubballoo3 left a comment

Choose a reason for hiding this comment

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

The code looks pretty good (besides one spelling error shown in the CI), but I am still confused on all the things 'stitching with backend' should entail. For example I can't submit workflows and have that trigger multiple jobs (it only runs the first one) but in the next PR we are already reporting on the job status of each node? Could you clear up what this code enables us to do beyond storing and reloading workflows (which it does quite well)

@Bubballoo3 Bubballoo3 moved this from Awaiting Review to Changes Requested in PR Review Pipeline Oct 27, 2025
@harshit-soora
Copy link
Contributor Author

I can't submit workflows and have that trigger multiple jobs (it only runs the first one) but in the next PR we are already reporting on the job status of each node

This current CL can trigger multiple jobs at once, the submit workflow button will submit graph (boxes and edges) to backend. Then from backend we are able to submit all jobs in order with the help of dag.rb class.

To test this you need to go to project's page and see how many jobs are running/queued.
image

@Bubballoo3
Copy link
Contributor

Bubballoo3 commented Oct 28, 2025

I think I might be confused, but here is the issue I was trying to explain in the last one. I have the following workflow,
image

with each test launcher printing a different message. When I submit the workflow, I get a single job that goes from queued, running, to completed, and an output script that reads the first message only. It never spawns additional jobs from the workflow. As far as I understand it, this should be working (as in it should submit all 4 jobs in the workflow), but it isn't. Is this something that you know and have planned for a future PR? or is this truly a bug?

UPDATE: This also applies to less complicated workflows, like having 5 launchers all in a row with a totally linear graph. Same behavior of the first script running and nothing else happening.

@Bubballoo3
Copy link
Contributor

In testing this further I caught another bug, which is that if you get impatient and select the submit button twice (before confirmation message) then you end up submitting two workflows. I think all of the new buttons here that depend on backend actions should disable while it waits for a response and then re-enable once the action has finished.

@harshit-soora
Copy link
Contributor Author

harshit-soora commented Oct 28, 2025

I rebased my test setup with the code from this github repo. But I am still not seeing the issue of other task not running except just first. Can you please share your development.log (ondemand/dev/dashboard/log/development.log). Make sure to arrive at the workflow page, clean the development log then hit submit and share whatever got entered in the development logs.

When I submitted the workflow:
image

In middle:
image

@harshit-soora
Copy link
Contributor Author

harshit-soora commented Oct 28, 2025

It will show something like this, where the highlighted part shows the order in which individual launchers are executed.

Also make sure that your backend has following files - (commit b51c1a6)
ondemand/apps/dashboard/app/models/dag.rb
ondemand/apps/dashboard/app/models/workflow.rb latest version
image

@Bubballoo3
Copy link
Contributor

Sorry this was my bad, the launchers were failing because they didn't have the account set, which makes me realize we probably want a place for global launcher settings, but that's besides the point. Fixing that launcher issue allowed the workflow to run in its entirety.

@harshit-soora
Copy link
Contributor Author

Alright np, I have fixed the double click issue.

@harshit-soora
Copy link
Contributor Author

I removed "status" parameter from json, as it wasn't used and neither needed for JS to get information around if the fetch request was processed or not.

While status: :not_found, status: :unprocessable_entity will help with HTTP error codes thus we have no need for separate status field.

Copy link
Contributor

@Bubballoo3 Bubballoo3 left a comment

Choose a reason for hiding this comment

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

I think this is good to go (will leave it unmerged until the end of the day again in case Jeff wants to weigh in).

I did discover an bug where clicking edit on a launcher (possibly other launcher buttons too) and then clicking the browser back button breaks the workflow javascript and shows a blank canvas. Clicking the rendered back button on the edit page takes you back to the project show page, so the browser back button is currently the only way to directly return to the workflow graph.

Anyway I am happy to merge this now and get this fixed in the next one considering the bug lies in the UI and not the backend, just wanted to get this on your radar.

@harshit-soora
Copy link
Contributor Author

Oh yes I know that bug, will keep it in my list and fix it afterwards.

@Bubballoo3 Bubballoo3 merged commit d7794a8 into OSC:master Oct 31, 2025
39 of 40 checks passed
@github-project-automation github-project-automation bot moved this from Changes Requested to Merged/Closed in PR Review Pipeline Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Merged/Closed

Development

Successfully merging this pull request may close these issues.

3 participants