Added job scheduling page #23

Med16-11 · 2023-06-25T18:49:51Z

screen-capture.mp4

This PR is dependent on the following PRs:

ceph/teuthology-api#51
ceph/teuthology#1924

Testing this PR

please checkout -b the PRs above for teuthology and teuthology-api.

And in teuthology-api do

diff --git a/setup.cfg b/setup.cfg
index babeb07..26154f1 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -56,7 +56,7 @@ install_requires =
     itsdangerous
     pydantic-settings
     python-dotenv
-    teuthology @ git+https://github.com/ceph/teuthology#egg=teuthology[test]
+    teuthology @ git+https://github.com/ceph/teuthology@wip-ksirivad-teuth-suite-exception

kamoltat · 2023-06-27T04:01:16Z

@Med16-11 always rebase against latest main branch before filing a PR, or else there will likely be merge conflicts like what you see right now.

kamoltat · 2023-06-27T04:04:25Z

src/pages/Schedule/index.js

+          </button>
+        </div>
+      </div>
+      {/* <div>


Please check for things like commented code before committing :)

Sure, I actually thought that we can use the commented code later to ensure that buttons are working properly and can write the logic in functions later.

kamoltat · 2023-06-29T16:49:38Z

Okay so a few things we can improve:

The key and value column should be a dynamic table where when you input data into the last row, it will automatically generate a new row.
key and value should be an input form ( currently it is static)
When hover on to a row, you should be able to delete it, similar to POSTMAN where a little trash icon will appear and you can just delete the row.
The top box which displays the teutholgoy-suite command should automatically generate the command based on the kv that is given in table.
run and dry-run button should have red and blue colors.

Med16-11 · 2023-06-30T08:50:06Z

Okay so a few things we can improve:

The key and value column should be a dynamic table where when you input data into the last row, it will automatically generate a new row.

key and value should be an input form ( currently it is static)

When hover on to a row, you should be able to delete it, similar to POSTMAN where a little trash icon will appear and you can just delete the row.

The top box which displays the teutholgoy-suite command should automatically generate the command based on the kv that is given in table.

run and dry-run button should have red and blue colors.

Got it.

Med16-11 · 2023-06-30T08:57:12Z

It seems like I have closed the PR accidentally. Could you please open it @kamoltat

kamoltat · 2023-07-10T14:10:38Z

Thanks you for filling the change,

here are some of the things I think can be improved:

The button, you should use import Button from "@mui/material/Button"; the color for run button can be used as color="error" for now. For dry-run button try to find a blue color that matches the color="error".
The command box only gets updated once you press the run button, what we should do is to have the command box update every-time we check/uncheck the box for each key. This also applies when we delete a row that is checked.
Instead of generating a new column for the delete row button, we can have a small trash/bin icon inside the value column for each row, similar to how POSTMAN did it.

Med16-11 · 2023-07-17T13:07:28Z

It looks like this now.

kamoltat

@Med16-11
Thanks for the change, I think with the table color being black it is hard to see in dark mode, might be worth also changing the border to white or light grey when in dark mode.

kamoltat · 2023-07-18T13:49:23Z

I realized that you have a merge commit, we can avoid that by doing:

git fetch upstream

git rebase upstream/main

Also, to keep the commits clean in your PR, you should try to squash your commits in to 1 commit whenever the change between the two commits serve the same purpose.

You can do this by:

commit your new change:
git add <file>
write some random commit message
git commit -s
Now lets say you have N new commits on top of main you do:
git rebase -HEAD~N

In the interactive shell put an s

pick 02efde5 resolved conflits
pick 1b725ad resolved other conflits
pick 9374fad added the suggested changes
pick dfd9165 pages/Job/index.js: fix job started time
pick 15386e1 Implement kill-run button using mock API endpoint
s 7a3edd9 new changes added

Then it will redirect you to commit message , just edit the commit by deleting the random commit message you wrote and keep/edit the original commit message that you a squashing to.

kamoltat · 2023-07-25T15:43:24Z

Pending merge conflict to be resolved

Med16-11 · 2023-07-25T18:17:54Z

Pending merge conflict to be resolved

On it.

Med16-11 · 2023-07-26T15:15:04Z

That's how table looks now in dark mode.

Med16-11 · 2023-07-26T15:17:28Z

I'll be squashing all the commits after receiving your reviews.

kamoltat · 2023-07-26T15:54:23Z

@Med16-11 Thanks for the table change the border looks good now for dark mode.

kamoltat · 2023-07-26T15:57:29Z

@Med16-11 I've noticed that:

The input box for the key column cannot be edited?
If we tick the checkbox for a bunch keys/values and delete it, the command box up top still retains the value of the deleted row.

Med16-11 · 2023-07-26T16:01:12Z

@Med16-11 I've noticed that:

The input box for the key column cannot be edited?

If we tick the checkbox for a bunch keys/values and delete it, the command box up top still retains the value of the deleted row.

Okay, I'll look into it.

kamoltat · 2023-08-02T16:00:01Z

@Med16-11 thanks for your new change,
you're very close,

so I think the key column is still unable to be inputed.
Trash Icon in dark mode cannot be seen since it is white and when the cursor is on the row, it becomes white as well

VallariAg · 2023-08-07T12:18:52Z

@kamoltat @zmc
The schema for /suite endpoint on teuthology-api is defined and restrictive. Since the API does not allow the freedom to add any other args to the teuthology-suite command, the KEY column's can be plain text (listing all possible args) instead of input fields. This would let the user know about the restrictions of key names. Otherwise we would need to check the key names before requesting teuthology-api and throw an error if its not supported.
What do you think?

@Med16-11
Good work! I know it's WIP, just letting you some of the issues:

If I edit a row after clicking the "check", the command on top does not edit.
Command input at top and the form below are out of sync if something is directly added to the command input. They need to be synced as it would cause confusion for the user about which args would be considered for the run.

zmc · 2023-08-07T20:45:36Z

@VallariAg If I understand you correctly, you're suggesting we not allow/require the user to input the flag names in the "Key" column, and instead list all the available options and allow the user to fill in the ones they need. I think I agree with that. We could populate a Select and allow adding items to the table from that list.

This reminds me: at some point we'll want to be populating default values; perhaps we should add an endpoint to teuthology-api that provides these.

VallariAg · 2023-08-08T03:42:11Z

@zmc that sounds perfect. I'll open an issue for adding an endpoint for default values on teuthology-api and using it on pulpito-ng.

In teuthology meeting, we also discussed to disable editing on the command input above the table. We can focus on scheduling runs with the form table for now. I'll open a new issue for later enabling that feature.

kamoltat · 2023-08-09T15:43:35Z

@zmc @VallariAg
Okay so I think I'm fine with having all the suite keys as an out of the box experience. But from a Teuthology user perspective I only use around 10 - 13 out of 50+ arguments. If I were to use pulpito-ng to schedule jobs I would probably like to have only 10-13 keys in my key column with like at least 2-3 keys being selected by default. Therefore, I think we should keep the key delete functionality and key add button.

kamoltat · 2023-12-01T16:45:40Z

Made some changes:

Rewrote the components with Material UI

Improved OnListener logics

Improved how we store data in useLocalStorage.

NOTE:

Currently, there is a bug in removing the row, when deleting the row, the command box doesn't immediately remove the key and value associated with the row that has been removed. It's not a logic issue, since if we look at the localStorage in the browser, the rowData array updates correctly. I suspect that because of the async nature of JS, the problem is in:
  const handleDeleteRow = (index) => {
    console.log("handleDeleteRow");
    const newRowData = [...rowData];
    newRowData.splice(index, 1);
    setRowData(newRowData);
    const updatedRowCount = rowCount - 1;
    setRowCount(updatedRowCount);
    updateCommandBar();
  };
where newRowData.splice(index, 1); and setRowData(newRowData); are still processing, while JS decides to run updateCommandBar(); first.

This is Fixed!

VallariAg

Looks really great! I liked the feature that on adding new row, it would have next key from keyOptions by default.

src/pages/Schedule/index.jsx

VallariAg · 2023-12-13T14:24:06Z

src/components/Drawer/index.jsx

-      <RouterLink to="/stats/nodes/lock" className={classes.drawerLink}>
-        Node Locks Stats
-      </RouterLink>
-      <RouterLink to="/stats/nodes/jobs" className={classes.drawerLink}>
-        Node Jobs Stats
-      </RouterLink>
-      <Divider />


I think this was maybe removed accidentally?

yep, let me add this back in my commit

Just Added back and squashed the commit to the first commit. I removed the Divider, don't think we need that.

Nice! We can also probably disable "Run" and "Dry Run" Buttons if the user is not logged in. Or show a warning on top of schedule page.

kamoltat · 2024-02-01T23:11:36Z

NOTE: I always forget to drop the eb58790
since the PR that has this commit hasn't been merged yet. So reviewer please help watch out for this hot reload commit.

Nevermind, the PR for that commit is merged so it is no longer cherry-picked to my dev local branch

kamoltat · 2024-02-14T15:08:26Z

In order, to test this PR properly, we need to modify the dry_run args in teuthology_api

diff --git a/src/teuthology_api/services/suite.py b/src/teuthology_api/services/suite.py
index 8f1d0f7..af29aaa 100644
--- a/src/teuthology_api/services/suite.py
+++ b/src/teuthology_api/services/suite.py
@@ -22,8 +22,8 @@ def run(args, dry_run: bool, send_logs: bool, access_token: str):
         )
     try:
         args["--timestamp"] = datetime.now().strftime("%Y-%m-%d_%H:%M:%S")
-        if dry_run:
-            args["--dry-run"] = True
+        if args["--dry-run"] == True:
+            log.info("This run is a dry run")
             logs = logs_run(teuthology.suite.main, args)
             return {"run": {}, "logs": logs}

(Edit: ceph/teuthology-api#48 resolved this)

Signed-off-by: Med16-11 <singhmedhavi923@gmail.com> resolved conflits

Signed-off-by: Med16-11 <singhmedhavi923@gmail.com>

Signed-off-by: Zack Cerza <zack@redhat.com>

1. Rewrote the components with Material UI 2. Improved OnListener logics 3. Improved how we store data in useLocalStorage. Signed-off-by: Kamoltat Sirivadhna <ksirivad@redhat.com>

…ling suites Use axios post request to for run, dry-run and force-priority Signed-off-by: Kamoltat Sirivadhna <ksirivad@redhat.com>

kamoltat · 2024-03-13T18:37:18Z

screen-capture.mp4

VallariAg · 2024-04-25T10:39:46Z

src/lib/teuthologyAPI.ts

+        console.log("Error: --user doesn't match username of current logged in account");
+        return false;
+    }
+    return await axios.post(url, commandValue, {


@kamoltat
In order to kill a run scheduled by pulpito-ng's schedule feature, teuthology would look for owner (which is set by "--owner" parameter). If no "--owner" is passed in teuthology-suite command, teuthology sets it as scheduled_<machine username>@<machine hostname>.
In teuthology-api and pulpito-ng setup, I think it'll work out well if we set commandValue['--owner'] = github_username on pulpito-ng before sending request to t-api. This way we would be able to recognise the owner as github username when we try to kill the run later.

Also, I think we can skip "--user" checks entirely since we merged https://github.com/ceph/teuthology-api/pull/47/files#diff-d64c10972091566c2a44e7e643a8e41b7a4781efb8cddb9516b0cfe320ff648f in which we set "--user" as github username in teuthology-api.

Thank you, new commit should address this.

kamoltat · 2024-05-10T20:38:29Z

ran into this error:

Run npm run lint

> pulpito-ng@0.1.0 lint
> eslint src

/home/runner/work/pulpito-ng/pulpito-ng/src/lib/teuthologyAPI.ts
  30:22  error  React Hook "useUserData" is called in function "doSchedule" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use"  react-hooks/rules-of-hooks

Error: Process completed with exit code 1.

Therefore, I decided to change the name doSchedule to useSchedule

schedule feature uses useMutation to deal with cases like onSuccess, onError and isLoading. CircularProgress is added when button is clicked and mutation is in the state of isLoading. Signed-off-by: Kamoltat Sirivadhna <ksirivad@redhat.com>

1. After a run is scheduled, user will be able to see the results including the logs from teuthology-suite. Only success runs will show all logs from teuthology-suite, failed runs will only return the exception with descriptive failure reasons. 2. Added Warning Alerts for 0 jobs scheduled in a run 3. Added more arguments that can be use to schedule run Signed-off-by: Kamoltat Sirivadhna <ksirivad@redhat.com>

kamoltat self-requested a review June 26, 2023 14:37

kamoltat assigned Med16-11 Jun 27, 2023

kamoltat added the feature New feature or request label Jun 27, 2023

kamoltat requested changes Jun 27, 2023

View reviewed changes

kamoltat linked an issue Jun 29, 2023 that may be closed by this pull request

Teuthology-suite #4

Open

4 tasks

Med16-11 closed this Jun 30, 2023

kamoltat reopened this Jun 30, 2023

kamoltat self-requested a review July 18, 2023 13:12

kamoltat requested changes Jul 18, 2023

View reviewed changes

This was referenced Aug 8, 2023

scheduling page: populate default values using new teuthology-api endpoint #41

Open

scheduling page: disable edit for teuthology-suite command input #42

Open

kamoltat force-pushed the medhavi branch 3 times, most recently from 6ee907f to 06e9202 Compare December 1, 2023 16:44

kamoltat requested a review from VallariAg December 4, 2023 15:38

VallariAg reviewed Dec 13, 2023

View reviewed changes

src/pages/Schedule/index.jsx Outdated Show resolved Hide resolved

src/pages/Schedule/index.jsx Outdated Show resolved Hide resolved

src/pages/Schedule/index.jsx Outdated Show resolved Hide resolved

src/pages/Schedule/index.jsx Show resolved Hide resolved

VallariAg reviewed Dec 13, 2023

View reviewed changes

kamoltat force-pushed the medhavi branch 2 times, most recently from 3957229 to 1b77720 Compare December 13, 2023 22:08

kamoltat force-pushed the medhavi branch from f30b55a to 9e02a60 Compare January 30, 2024 20:18

kamoltat force-pushed the medhavi branch 2 times, most recently from 5cefc09 to aff5afd Compare February 14, 2024 15:06

kamoltat force-pushed the medhavi branch from aff5afd to 8a521a9 Compare February 14, 2024 17:04

Med16-11 and others added 5 commits March 7, 2024 14:24

Added job scheduling page

20daee2

Signed-off-by: Med16-11 <singhmedhavi923@gmail.com> resolved conflits

lock & unlock widget added

ee23161

Signed-off-by: Med16-11 <singhmedhavi923@gmail.com>

App: Add route for Schedule

564c1b9

Signed-off-by: Zack Cerza <zack@redhat.com>

src/pages/Schedule/index: UI facelift + improve logic

aedaec8

1. Rewrote the components with Material UI 2. Improved OnListener logics 3. Improved how we store data in useLocalStorage. Signed-off-by: Kamoltat Sirivadhna <ksirivad@redhat.com>

src/pages/Schedule: Connect pulpito-ng with teuthology-api for schedu…

9941df5

…ling suites Use axios post request to for run, dry-run and force-priority Signed-off-by: Kamoltat Sirivadhna <ksirivad@redhat.com>

kamoltat force-pushed the medhavi branch from 8a521a9 to d947c73 Compare March 13, 2024 20:51

This was referenced Mar 13, 2024

teuthology/suite/run.py: Improve ScheduleFail exception ceph/teuthology#1924

Open

src/teuthology_api/suite: Modify how we handle Error and Success runs ceph/teuthology-api#51

Open

VallariAg reviewed Apr 25, 2024

View reviewed changes

kamoltat force-pushed the medhavi branch from d947c73 to 8a858fb Compare May 8, 2024 15:39

kamoltat added 2 commits May 10, 2024 16:45

kamoltat force-pushed the medhavi branch from 8a858fb to d876adc Compare May 10, 2024 20:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added job scheduling page #23

Added job scheduling page #23

Med16-11 commented Jun 25, 2023 •

edited by kamoltat

kamoltat commented Jun 27, 2023

kamoltat Jun 27, 2023

Med16-11 Jun 28, 2023

kamoltat commented Jun 29, 2023

Med16-11 commented Jun 30, 2023 •

edited

Med16-11 commented Jun 30, 2023

kamoltat commented Jul 10, 2023

Med16-11 commented Jul 17, 2023

kamoltat left a comment

kamoltat commented Jul 18, 2023 •

edited

kamoltat commented Jul 25, 2023

Med16-11 commented Jul 25, 2023

Med16-11 commented Jul 26, 2023

Med16-11 commented Jul 26, 2023 •

edited

kamoltat commented Jul 26, 2023

kamoltat commented Jul 26, 2023

Med16-11 commented Jul 26, 2023

kamoltat commented Aug 2, 2023

VallariAg commented Aug 7, 2023

zmc commented Aug 7, 2023

VallariAg commented Aug 8, 2023

kamoltat commented Aug 9, 2023 •

edited

kamoltat commented Dec 1, 2023

VallariAg left a comment

VallariAg Dec 13, 2023

kamoltat Dec 13, 2023

kamoltat Dec 13, 2023

VallariAg Dec 14, 2023

kamoltat commented Feb 1, 2024 •

edited

kamoltat commented Feb 14, 2024 •

edited

kamoltat commented Mar 13, 2024

VallariAg Apr 25, 2024

kamoltat May 8, 2024

kamoltat commented May 10, 2024 •

edited

Added job scheduling page #23

Are you sure you want to change the base?

Added job scheduling page #23

Conversation

Med16-11 commented Jun 25, 2023 • edited by kamoltat

Testing this PR

kamoltat commented Jun 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamoltat commented Jun 29, 2023

Med16-11 commented Jun 30, 2023 • edited

Med16-11 commented Jun 30, 2023

kamoltat commented Jul 10, 2023

Med16-11 commented Jul 17, 2023

kamoltat left a comment

Choose a reason for hiding this comment

kamoltat commented Jul 18, 2023 • edited

kamoltat commented Jul 25, 2023

Med16-11 commented Jul 25, 2023

Med16-11 commented Jul 26, 2023

Med16-11 commented Jul 26, 2023 • edited

kamoltat commented Jul 26, 2023

kamoltat commented Jul 26, 2023

Med16-11 commented Jul 26, 2023

kamoltat commented Aug 2, 2023

VallariAg commented Aug 7, 2023

zmc commented Aug 7, 2023

VallariAg commented Aug 8, 2023

kamoltat commented Aug 9, 2023 • edited

kamoltat commented Dec 1, 2023

VallariAg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamoltat commented Feb 1, 2024 • edited

kamoltat commented Feb 14, 2024 • edited

kamoltat commented Mar 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamoltat commented May 10, 2024 • edited

Med16-11 commented Jun 25, 2023 •

edited by kamoltat

Med16-11 commented Jun 30, 2023 •

edited

kamoltat commented Jul 18, 2023 •

edited

Med16-11 commented Jul 26, 2023 •

edited

kamoltat commented Aug 9, 2023 •

edited

kamoltat commented Feb 1, 2024 •

edited

kamoltat commented Feb 14, 2024 •

edited

kamoltat commented May 10, 2024 •

edited