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.

Sign up for GitHub

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

Jump to bottom

Include cancelling long running jobs #596

Closed
wants to merge 22 commits into from
Closed

Include cancelling long running jobs #596

wants to merge 22 commits into from

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
soxofaan
soxofaan reviewed Jul 19, 2024
View reviewed changes
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
soxofaan
soxofaan reviewed Jul 22, 2024
View reviewed changes
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

soxofaan
soxofaan reviewed Jul 26, 2024
View reviewed changes
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
soxofaan
soxofaan reviewed Aug 19, 2024
View reviewed changes
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
HansVRP and others added 15 commits August 20, 2024 14:45
@HansVRP @soxofaan
Update openeo/extra/job_management.py
ad0cd43 
Co-authored-by: Stefaan Lippens <[email protected]>
@HansVRP @soxofaan
Update openeo/extra/job_management.py
d1fd26c 
Co-authored-by: Stefaan Lippens <[email protected]>
@HansVRP @soxofaan
Update openeo/extra/job_management.py
788496a 
Co-authored-by: Stefaan Lippens <[email protected]>
@HansVRP @soxofaan
Update openeo/extra/job_management.py
89b1e36 
Co-authored-by: Stefaan Lippens <[email protected]>
@HansVRP @soxofaan
Update openeo/extra/job_management.py
0b353c0 
Co-authored-by: Stefaan Lippens <[email protected]>
@HansVRP @soxofaan
Update openeo/extra/job_management.py
6828ca8 
Co-authored-by: Stefaan Lippens <[email protected]>
@HansVRP @soxofaan
Update openeo/extra/job_management.py
de9ce04 
Co-authored-by: Stefaan Lippens <[email protected]>
@HansVRP @soxofaan
Update openeo/extra/job_management.py
b5a10bb 
Co-authored-by: Stefaan Lippens <[email protected]>
@HansVRP
code review changes
fdbc1c9
@HansVRP
Merge branch 'hv_prolonged_runs' of https://github.com/Open-EO/openeo…
ba41901 
…-python-client into hv_prolonged_runs
@HansVRP
Merge branch 'master' into hv_prolonged_runs
5960311
@HansVRP
clean up
8fa5dfb
@HansVRP
avoid infinite loop
3d89e4d
@HansVRP
uniformly use rfc3339.utcnow()
eec293f
@HansVRP
clean up
335f85a
@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 soxofaan linked an issue Sep 3, 2024 that may be closed by this pull request
As a developer I want to provide the MultiBackendJobManager with a maximal run time for each job. #590
Closed
soxofaan pushed a commit that referenced this pull request Sep 3, 2024
@HansVRP @soxofaan
Squashed commit of PR #596 (cancel feature in job manager)
b656fd7 
refs: #596, #590
soxofaan added a commit that referenced this pull request Sep 3, 2024
@soxofaan
PR #596: Further finetuning and changelog
d47de26
@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
@soxofaan
PR #596 finetune tests for cancel feature of job manager
3ee39de 
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
@soxofaan
Skip some job manager tests on python 3.7
48fd057 
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
@soxofaan
PR #596 finetune tests for cancel feature of job manager
03a8cc6 
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
@soxofaan
Skip some job manager tests on python 3.7
b50ea55 
no support for `shift` on `time_machine` fixture below python 3.8

refs #590, #596, #578
@soxofaan soxofaan mentioned this pull request Sep 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soxofaan soxofaan soxofaan left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
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
@HansVRP @jdries @soxofaan