search

golemfactory / clay

Golem is creating a global market for computing power.
https://golem.network
GNU General Public License v3.0
2.91k stars 286 forks source link

Golemcli tasks abort TaskID - not working #4673

Closed cryptobench closed 5 years ago

cryptobench commented 5 years ago

Description

Trying to abort a task using the CLI results in an error. Golem Version: 0.20.2 Golem-Messages version (leave empty if unsure):

Electron version (if used): latest OS [e.g. Windows 10 Pro]: Mac OS Branch (if launched from source):

Mainnet/Testnet: Testnet Priority label is set to the lowest by default. To setup higher priority please change the label P0 label is set for Severity-Critical/Effort-easy P1 label is set for Severity-Critical/Effort-hard P2 label is set for Severity-Low/ Effort-easy P3 label is set for Severity-Low/Effort-hard

Description of the issue: Trying to abort a task that is currently running using golemcli tasks abort TaskID results in an error saying ERROR [golemcli] wamp.error.runtime_error: wamp.error.runtime_error

Screenshots: image

If applicable, add screenshots to help explain your problem.

Steps To Reproduce

  1. Launch task
  2. Open terminal and abort task using golemcli tasks abort TaskID
  3. Observe error.

Expected behavior

Golem cancels the task.

Logs and any additional context

Golemlogs.zip

Proposed Solution?

(Optional: What could be a solution for that issue)

shadeofblue commented 5 years ago

@cryptobench what kind of a task was that?

cryptobench commented 5 years ago

@shadeofblue Just wasm.

shadeofblue commented 5 years ago

@cryptobench thanks

mbenke commented 5 years ago

This may be the reason: https://github.com/golemfactory/golem/blob/0.20.2/apps/wasm/task.py#L424

def abort(self):
        raise NotImplementedError()
cryptobench commented 5 years ago

Definitely looks and sounds like it, haha!

jiivan commented 5 years ago

This may be the reason: https://github.com/golemfactory/golem/blob/0.20.2/apps/wasm/task.py#L424

def abort(self):
        raise NotImplementedError()

IMHO it would be nice to handle this gracefully in CLI. I.e. to:

  1. Handle not NotImplemented error as a special case
  2. Handle all other errors with a more readable way
mplebanski commented 5 years ago

An actual fix for that is simply not overriding abort. There is more logic to aborting to be found in TaskServer and abort at task level is a simple hook that can e.g. do nothing. The problem here is that is raises an exception insteading doing nothing...