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

Tasks should be marked as "completed" when killed by the RPC server #1838

Closed
drewbanin opened this issue Oct 16, 2019 · 2 comments · Fixed by #1837
Closed

Tasks should be marked as "completed" when killed by the RPC server #1838

drewbanin opened this issue Oct 16, 2019 · 2 comments · Fixed by #1837
Assignees
Labels
bug Something isn't working rpc Issues related to dbt's RPC server

Comments

@drewbanin
Copy link
Contributor

Describe the bug

When a task is killed using the kill method of the rpc server, the task is correctly terminated, but the status remains running in the output of the poll and ps methods. Instead, the rpc server should show the status as completed.

The poll method contains a field, response['result']['status'] which appears to always return "success" (at least for dbt run-type tasks). Can we instead make this field only return success when tasks have completed successfully? We can use the existing interpret_results() logic to determine this status for completed tasks?

Similar for entries in the ps output -- it appears that state is always success -- we should also make that reflect the actual status of the task.

Steps To Reproduce

  1. create a model with the contents
{{ config(materialized='table') }}
select 1 / 0 as id
  1. run the rpc server
  2. run this model in the rpc server
  3. kill the spawned task
  4. inspect the ps and poll output in the rpc server
@drewbanin drewbanin added bug Something isn't working rpc Issues related to dbt's RPC server labels Oct 16, 2019
@drewbanin drewbanin added this to the Louisa May Alcott milestone Oct 16, 2019
@beckjake
Copy link
Contributor

Ok, I'm a bit confused here... When I do this:

  • the run query successfully completes before I can really kill the project
  • the state in ps and poll is "success"

If I do a run_sql(sql="select * from pg_sleep(100)") and then kill it while running, the state in poll returns an error object and ps reports a status of "error".

Potentially what you're seeing is that if you kill a completed task, we don't mark it as "killed" (your response to kill will indicate that it's finished, not killed).

But in general, this behavior all seems good and correct to me? We can add a killed state to the task and tag killed stuff with it instead of the current "error", but if your task has finished before you kill it, the task state will definitely not be killed...

@drewbanin
Copy link
Contributor Author

@beckjake sorry - I got my wires crossed when making this issue. You should definitely use a model with contents like select 1 from pg_sleep(100), but make sure to materialize it as a table (a view will return immediately).

If you run this model with:

{
	"jsonrpc": "2.0",
	"method": "cli_args",
	"id": "123",
	"params": {
		"cli": "run --models debug",
	}
}

and then kill this task, you should find that the status in both the ps output is and poll output for this task is running. So, first and foremost, the rpc server should not show that this task is running because it is not running anymore.

In the case where tasks fail (either because they were killed, or they ran to completion but the resources executed by dbt failed/errored in some way), the rpc server should indicate that the result of the task was not successful, eg. by setting status or state to error. If we alternatively want to make the status something like "complete", but also include a separate field which denotes that manner in which it completed (success or error) we can certainly do that too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rpc Issues related to dbt's RPC server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants