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

bug fixes and more #24

Merged
merged 17 commits into from
Apr 14, 2023
Merged

bug fixes and more #24

merged 17 commits into from
Apr 14, 2023

Conversation

bertsky
Copy link
Member

@bertsky bertsky commented Apr 11, 2023

  • process_id must be a str not an int, esp. for process_mets.sh
  • active job status must use PID from $REMOTEDIR/ocrd.pid (on Controller), not from $PID (on Manager)
  • add missing job status D
  • jobs: show (and sort by) time created/terminated
  • use /workflows volume
  • active jobs: add kill button (dummy for now)

@bertsky bertsky requested a review from SvenMarcus April 11, 2023 14:45
@bertsky
Copy link
Member Author

bertsky commented Apr 11, 2023

needs slub/ocrd_manager#57

@bertsky
Copy link
Member Author

bertsky commented Apr 11, 2023

  • add kill button (dummy for now)

@SvenMarcus to implement this, we would need to "log in" to the Manager and kill the given process ID. But I don't like the Monitor to hold the keys. Perhaps we should instead start implementing our minimalistic web server in the Manager itself – delegating to for_production.sh, for_presentation.sh (which we will need for new jobs anyway) and to the killer.

Sry, I'll update the CI tests shortly.

ocrdmonitor/processstatus.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@markusweigelt markusweigelt left a comment

Choose a reason for hiding this comment

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

Great work thx!

With the test, we are currently running on the CI in an endless loop.

tests/ocrdmonitor/test_readlogs.py .....                                 [ 37%]
tests/ocrdmonitor/test_redirect.py .........                             [ [53](https://github.com/slub/ocrd_monitor/actions/runs/4677162691/jobs/8284349346#step:5:54)%]
Error: The operation was canceled.

To me it looks like the tests/ocrdmonitor/test_sshps.py is affected.
Possibly it is due to the changed SSH user, since the test itself was not adjusted.

@SvenMarcus What do you think?

init.sh Show resolved Hide resolved
ocrdmonitor/server/templates/jobs.html.j2 Show resolved Hide resolved
@markusweigelt
Copy link
Collaborator

markusweigelt commented Apr 13, 2023

All right thanks. When the pytest problem is solved I give my approve.

@SvenMarcus SvenMarcus dismissed markusweigelt’s stale review April 14, 2023 14:34

Everything except naming of ProcessState should be resolved

@SvenMarcus SvenMarcus merged commit 2fe3b2d into main Apr 14, 2023
@SvenMarcus SvenMarcus deleted the fix-jobinfo branch April 14, 2023 14:42
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.

3 participants