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

feat: reload job status every 5 sec. and show monitoring message #110

Merged
merged 3 commits into from
Jun 5, 2024

Conversation

dmijatovic
Copy link
Contributor

@dmijatovic dmijatovic commented May 24, 2024

Reload job status

Closes #72

Changes made:

  • Add effect to reload job status after specified time (in ms). The time is stored in env variable HADDOCK3WEBAPP_REFRESH_RATE_MS. The default timeout is 10 sec.
  • Show message "Monitoring state change..."
  • Created DotsLoader component to show the message
  • Added "Last checked on" to indicate the time when the last state was checked
  • Not related: increased width of workflow section to fit all buttons in one row

Example

image

@dmijatovic dmijatovic requested a review from sverhoeven May 24, 2024 16:31
…en workflow section to fit all buttons in one row.
@dmijatovic dmijatovic force-pushed the 72-refresh-job-page branch from abc3800 to 0919d7d Compare May 26, 2024 16:59
@dmijatovic dmijatovic changed the title feat: reload job status every 5 sec. ans show monitoring message feat: reload job status every 5 sec. and show monitoring message May 26, 2024
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

I tried it with
12.zip which took 13 minutes on my machine and worked as expected.

We need to find the balance between refreshing enough times for the user and not stress the machine with all the polling for like a 100 queued jobs. It also depends on the deployment, it would be nice if the refresh rate could be set with an env var like HADDOCK3WEBAPP_REFRESH_RATE _MS.

app/bartender-client/constants.ts Outdated Show resolved Hide resolved
app/routes/jobs.$id._index.tsx Outdated Show resolved Hide resolved
app/routes/jobs.$id._index.tsx Outdated Show resolved Hide resolved
app/components/ui/DotsLoader.tsx Show resolved Hide resolved
@dmijatovic dmijatovic force-pushed the 72-refresh-job-page branch from 994e7d4 to 89fe21b Compare May 30, 2024 11:45
@dmijatovic
Copy link
Contributor Author

@sverhoeven I have refactored code to use env variable HADDOCK3WEBAPP_REFRESH_RATE_MS.
The variable is added to .env file in the deploy/arq.
Let me know if you have any other suggestions

@dmijatovic dmijatovic requested a review from sverhoeven May 30, 2024 11:48
@dmijatovic dmijatovic force-pushed the 72-refresh-job-page branch 2 times, most recently from 82ce128 to 1ebe2c7 Compare May 30, 2024 11:52
refactor: add maxLenght as optional param to DotsLoader component
@dmijatovic dmijatovic force-pushed the 72-refresh-job-page branch from 1ebe2c7 to df7d90a Compare May 31, 2024 19:15
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

Tested it out and works as expected.

Split of #113 for later.

@dmijatovic dmijatovic force-pushed the 72-refresh-job-page branch from bcbce4b to 63659b7 Compare June 4, 2024 11:02
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

Just tried it out. Last checked on field update every 30s. While the dots (dis)appear every second.

Ok to merge

@dmijatovic dmijatovic merged commit ea3e8ca into main Jun 5, 2024
2 checks passed
Copy link

github-actions bot commented Jun 5, 2024

Please delete the images belonging to this Pull Request as they are no longer useful.

Goto versions page of each image, find version called pr-110, click on ... button and select Delete version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto refresh job page
2 participants