Bug 1544403 Comment 7 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Pete Moore [:pmoore][:pete] from comment #6)

> Note, the current implementation won't easily allow other exit codes to be added to the list - ideally we'd implement this the same way we have in docker-worker, e.g. [here](https://dxr.mozilla.org/mozilla-central/rev/c7a22f0ea7b4ce3ff379f5be1d1ad3070aa76317/taskcluster/taskgraph/transforms/task.py#570-571,572).
> 
> Although maybe we can do that in a subsequent bug, in order not to block this one.

here's an attempt at what i think was suggested:
```
    task_def['payload'] = {
        'command': worker['command'],
        'maxRunTime': worker['max-run-time'],
        'onExitStatus': {},
    }

    if worker['os'] == 'windows':
        worker.setdefault('retry-exit-status', []).extend(
            # these codes (on windows) indicate a process interruption,
            # rather than a task run failure. see bug 1544403.
            1073807364, # process force-killed due to system shutdown
            3221225786, # sigint (any interrupt)
        )
    if 'retry-exit-status' in worker:
        task_def['payload']['onExitStatus']['retry'] = worker['retry-exit-status']
```
instead of what was implemented in the patch:
```
    task_def['payload'] = {
        'command': worker['command'],
        'maxRunTime': worker['max-run-time'],
        'onExitStatus': {
            'retry': [
                # these codes (on windows) indicate a process interruption,
                # rather than a task run failure. see bug 1544403.
                1073807364, # process force-killed due to system shutdown
                3221225786, # sigint (any interrupt)
            ],
        },
    }
```
i'm of the fairly strong opinion that the second of these two implementations is cleaner. i'm not sure what was meant by "won't easily allow other exit codes to be added to the list". please clarify that. possibly with a code example here as i must have failed to comprehend that. i think it's pretty much exactly the same amount of effort to add exit codes to either implementation.
(In reply to Pete Moore [:pmoore][:pete] from comment #6)

> Note, the current implementation won't easily allow other exit codes to be added to the list - ideally we'd implement this the same way we have in docker-worker, e.g. [here](https://dxr.mozilla.org/mozilla-central/rev/c7a22f0ea7b4ce3ff379f5be1d1ad3070aa76317/taskcluster/taskgraph/transforms/task.py#570-571,572).
> 
> Although maybe we can do that in a subsequent bug, in order not to block this one.

here's an attempt at what i think was suggested:
```python
    task_def['payload'] = {
        'command': worker['command'],
        'maxRunTime': worker['max-run-time'],
        'onExitStatus': {},
    }

    if worker['os'] == 'windows':
        worker.setdefault('retry-exit-status', []).extend(
            # these codes (on windows) indicate a process interruption,
            # rather than a task run failure. see bug 1544403.
            1073807364, # process force-killed due to system shutdown
            3221225786, # sigint (any interrupt)
        )
    if 'retry-exit-status' in worker:
        task_def['payload']['onExitStatus']['retry'] = worker['retry-exit-status']
```
instead of what was implemented in the patch:
```python
    task_def['payload'] = {
        'command': worker['command'],
        'maxRunTime': worker['max-run-time'],
        'onExitStatus': {
            'retry': [
                # these codes (on windows) indicate a process interruption,
                # rather than a task run failure. see bug 1544403.
                1073807364, # process force-killed due to system shutdown
                3221225786, # sigint (any interrupt)
            ],
        },
    }
```
i'm of the fairly strong opinion that the second of these two implementations is cleaner. i'm not sure what was meant by "won't easily allow other exit codes to be added to the list". please clarify that. possibly with a code example here as i must have failed to comprehend that. i think it's pretty much exactly the same amount of effort to add exit codes to either implementation.

Back to Bug 1544403 Comment 7