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

Include cancelling long running jobs #596

Closed
wants to merge 22 commits into from
Closed

Conversation

HansVRP
Copy link
Contributor

@HansVRP HansVRP commented Jul 19, 2024

Included _check_and_stop_long_running_jobs.

Still needs to be covered in test_suite

@HansVRP HansVRP requested a review from soxofaan July 19, 2024 12:46
Copy link
Member

@soxofaan soxofaan left a comment

Choose a reason for hiding this comment

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

some initial notes

openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
@HansVRP
Copy link
Contributor Author

HansVRP commented Jul 26, 2024

@soxofaan I have worked on the comments above and also included 2 additional unit tests to test the behavior

Copy link
Member

@soxofaan soxofaan left a comment

Choose a reason for hiding this comment

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

some more quick notes 😄
(I haven't looked into tests yet)

openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
Copy link
Member

@soxofaan soxofaan left a comment

Choose a reason for hiding this comment

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

some more notes

openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
openeo/extra/job_management.py Outdated Show resolved Hide resolved
tests/extra/test_job_management.py Outdated Show resolved Hide resolved
tests/extra/test_job_management.py Outdated Show resolved Hide resolved
@jdries
Copy link
Collaborator

jdries commented Sep 2, 2024

Can we merge now? We have some other changes coming up as well on this one.

@HansVRP
Copy link
Contributor Author

HansVRP commented Sep 2, 2024

@soxofaan The issues seems to be resolved

@soxofaan
Copy link
Member

soxofaan commented Sep 3, 2024

squashed and merged in 712a6fa 👍

@soxofaan soxofaan closed this Sep 3, 2024
@soxofaan soxofaan deleted the hv_prolonged_runs branch September 3, 2024 22:49
soxofaan added a commit that referenced this pull request Sep 3, 2024
Call public `run_jobs` instead of internal `_track_statuses`.
Use a fake backend that interacts with standard requests, instead of mocking internals.
Parameterize `cancel_after_seconds` instead of separate test functions
soxofaan added a commit that referenced this pull request Sep 4, 2024
no support for `shift` on `time_machine` fixture below python 3.8

refs #590, #596, #578
soxofaan added a commit that referenced this pull request Sep 5, 2024
Call public `run_jobs` instead of internal `_track_statuses`.
Use a fake backend that interacts with standard requests, instead of mocking internals.
Parameterize `cancel_after_seconds` instead of separate test functions

refs #590, #596
soxofaan added a commit that referenced this pull request Sep 5, 2024
no support for `shift` on `time_machine` fixture below python 3.8

refs #590, #596, #578
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.

As a developer I want to provide the MultiBackendJobManager with a maximal run time for each job.
3 participants