Conversation
wvengen
requested changes
Feb 17, 2026
Member
wvengen
left a comment
wvengen
left a comment
There was a problem hiding this comment.
Well done!
One question to look into: the failing test does matter here.
| start_time, end_time = jobinfo.pop('start_time'), jobinfo.pop('end_time') | ||
| assert datetime.strptime(start_time, '%Y-%m-%d %H:%M:%S.%f') | ||
| assert end_time is None | ||
| assert datetime.strptime(end_time, '%Y-%m-%d %H:%M:%S.%f') |
Member
There was a problem hiding this comment.
This is the job cancel scenario: it seems here that for Kubernetes, the job end time is not available when the job is canceled. For Docker, it seems that it is available. So this is a behaviour difference between the two, and that is not desired.
Can you look at scrapyd to see what it emits? We want to mirror that behaviour (as far as possible).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #40
If a docker containers finishes, its attributes will contain a field FinishedAt.
When /listjobs.json is called, the function api_listjobs() in api.py is called. This function in turn will call the listjobs() function in docker.py. In this function a docker client is called to list all processes. The docker library function does the same as
docker ps -a. Then each of these jobs are parsed by the _parse_job() function. Here certain labels and attributes are grabbed and set. Since the FinishedAt field is available, we can grab this and use it to set the end_time field.To test the functionality the github workflows setup the environment and run tests when pushed to main or a pr is created. The test-docker part succeeds, but the manifest and k8 part fail on the last test
test_scenario_cancel_running_finished_ok. The test-manifest step fails somewhere unrelated to to changed files, so more research is needed as to why the assert fails. The test-k8s does fail on a related part,assert datetime.strptime(end_time, '%Y-%m-%d %H:%M:%S.%f'). This logic however is unchanged, so still unclear what the reason for this is. Previously the assert wasassert end_time is None, however I suspect that this assert was wrong. After a job is finished the end_time should be available. The assert probably fails, because the line'end_time': format_datetime_object(job.status.completion_time) if job.status.completion_time and state == 'finished' else None,in k8s.py. The end time doesn't just get set if the state is set to finished, but also ifjob.status.completion_time.