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

Improved implementation to kill a job #37

Merged
merged 10 commits into from
Aug 29, 2023
Merged

Improved implementation to kill a job #37

merged 10 commits into from
Aug 29, 2023

Conversation

markusweigelt
Copy link
Collaborator

The previous implementation was not sufficient for killing jobs.

The invocation using the host ocrd-managerdid not work, because the browser sends requests from the host, and here localhost was the correct host.

However, this leads to a CORS problem in the browser.

Therefore, the monitor was extended with the URL /jobs/kill/%PID% and sends an HTTP request to the manager. From the monitor's perspective, the manager can be found under the host ocrd-manager.

The URL to the manager's endpoint is passed to the monitor as an environment variable when it starts.

@markusweigelt markusweigelt linked an issue Aug 17, 2023 that may be closed by this pull request
ocrdmonitor/server/jobs.py Outdated Show resolved Hide resolved
ocrdmonitor/server/jobs.py Outdated Show resolved Hide resolved
ocrdmonitor/server/templates/jobs.html.j2 Show resolved Hide resolved
ocrdmonitor/server/templates/jobs.html.j2 Outdated Show resolved Hide resolved
ocrdmonitor/server/jobs.py Outdated Show resolved Hide resolved
@bertsky
Copy link
Member

bertsky commented Aug 17, 2023

The invocation using the host ocrd-managerdid not work, because the browser sends requests from the host, and here localhost was the correct host.

However, this leads to a CORS problem in the browser.

Good catch!

@markusweigelt markusweigelt merged commit 35689c0 into main Aug 29, 2023
@markusweigelt markusweigelt deleted the kill-job branch August 29, 2023 14:19
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.

Configurable Manager endpoint
3 participants