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

Proposal for extended task API #45980

Closed
dbaeumer opened this issue Mar 16, 2018 · 26 comments
Closed

Proposal for extended task API #45980

dbaeumer opened this issue Mar 16, 2018 · 26 comments
Assignees
Labels
api-proposal tasks Task system issues verified Verification succeeded
Milestone

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Mar 16, 2018

For this milestone we would like to add API that offers the following features:

  • query the available tasks
  • execute a task
  • terminate a task
  • have events for task execution and termination
  • allow to execute a task that is not managed by VS Code. For those tasks allow to scan the output.
  • allow to provide a task execution provider so that a task can be spawned on a different machine and is not execute in the terminal.

The new API should be homogeneous. That is why I try to handle it in one issue:

Query available tasks

This got implemented by proving a fetchTasks function on the workspace namespace:

export function fetchTasks(): Thenable<Task[]>;

Execute a Task

/**
 * Executes a task that is managed by VS Code. The returned
 * task handle can be used to terminate the task.
 *
 * @param task the task to execute
 */
export function executeTask(task: Task): Thenable<TaskExecution>;

The execute method returns a TaskExecution which is different form a Task for the following reason: there are requests that tasks can be started n times hence we need a different representation for a running task than for the task itself. A TaskExecution is currently opaque and doesn't offer any special properties. This might change in the future

Terminate Task

Straight forward. A function terminateTask in the workspace namespace that takes a TaskExecution

export function terminateTask(task: TaskExecution): void;

onDidStartTask

An Event on the workspace namespace firing when a task gets executed. The event type will be ExecuteTaskEvent with a running task property. Open question is whether the provided RunningTask instance can be used to terminate the task since the start could have happened from another extension. I would for now allow this.

interface TaskStartEvent {
    execution: TaskExecution;
}
export const onDidStartTask: Event<TaskStartEvent>;

OnDidEndTask

An event on the workspace namespace firing when a task terminates. Event type will be TaskTerminateEvent with a single RunningTask

interface TaskEndEvent {
    execution: TaskExecution;
}
export const onDidTerminateTask: Event<TaskEndEvent>;

Execute a task that is not managed by VS Code

Extension can simply construct the task and execute it using the workspace API. No access to the output is provided right now since this should go hand in hand of providing output of the underlying terminal.

Task Execution Provider

Under construction. Need to think a little more :-)

@vscodebot vscodebot bot added the tasks Task system issues label Mar 16, 2018
@dbaeumer
Copy link
Member Author

@jrieken FYI

@alpaix
Copy link

alpaix commented Mar 23, 2018

I'd like to share the first feedback on the proposed API based on a prototype work for a task state synchronization mechanism:

  • It'd be great to have a notification on task collection changes. For instance, when a user decides to add a new task, we'd apply this change automatically to all participants in the session.
  • TaskExecution interface doesn't have any identifiers. Therefore it's impossible to correlate an execution event with the actual task. It is necessary for task state synchronization.
  • Internally the implementation seems to define IDs for tasks and task executions. Is it possible to promote the IDs to public properties? It'd be easier to use string IDs for task lookup and serialization.
  • Is it possible to add a method to get a task by ID or name? A simple lookup method would help us avoiding task enumeration.
  • It's not possible currently to get the current task state. For our scenario, we need to know which tasks are running at that moment. It'd be very helpful to have a query or a task attribute.

Thanks!

@dbaeumer
Copy link
Member Author

@alpaix thanks for the feedback. Regarding your items:

  1. I didn't add this since I don't know it :-). This information is only available if a task comes from a tasks.json file but not if contributed by an extension. The task providers currently work like completion item providers. Whenever we need the information (for example to show in a selection box) we query them. Tasks are not cached on the VS Code core side. To add this we would need to tell task providers to signal new tasks using a change event. Can you elaborate a little more why this is necessary ?
  2. this is on purpose since I first wanted to understand what clients need to know. The easiest would be to have a property task which give access to the underlying task.
  3. I am very very reluctant to expose this since I can't ensure that the id is stable. For example if tasks get moved around in the tasks.json file they do get a different id. It is an internal implementation detail.
  4. this can be added but only for a task by name, type definition or the string identifier used in the tasks.json (this is not the internal _id).
  5. Having a query to get all active tasks is no problem and can be added.

@dbaeumer
Copy link
Member Author

I implemented the following changes for this milestone:

  • the TaskExecution has a property task which references the task that got started.
  • removed the terminateTask function and provided a TaskExecution#terminate now that the execution has properties. This makes it better extensible without polluting the global namespace

Did not do anything about 4, 5. Will do early next milestone.

@dbaeumer dbaeumer mentioned this issue Mar 26, 2018
2 tasks
@ThadHouse
Copy link

Would it be possible to query the stdout and stderr of a task from an extension, either during the execution with an event or afterward? Either one works, but it would enable some more advanced problem matchers, and possibility to use tasks instead of custom exec node commands.

@alpaix
Copy link

alpaix commented Mar 26, 2018

@dbaeumer Thanks for addressing the feedback. Here are some thoughts:

  1. I planned to cache tasks on a remote instance to avoid querying the host every time and reduce latency. This is merely an optimization feature I can proceed without it.
  2. New Task property on the TaskExecution is exactly what I needed to relate it to the original task.
  3. I'm planning to create a collection of shadow tasks in a remote VSCode instance and I have to find a method of linking them back to original tasks. It's okay if the ID changes this way I could track the changes on my end. Otherwise, I'll have to introduce my own IDs to map the actual tasks.
  4. Look up by a name would be sufficient as long as the name of the Task is guaranteed to be unique.

@alpaix
Copy link

alpaix commented Mar 26, 2018

I'm also interested in capturing stdout of a task. @dbaeumer Did you consider such functionality as part of the Task Execution Provider?

@dbaeumer
Copy link
Member Author

Regarding stdin / stdout: there are plans to provide these stream however this has to be something that goes hand in hand how a terminal will expose it, which we started to discuss as well. Since tasks are executed in a terminal in doesn't make too much sense if a task comes up with its own story here.

@dbaeumer
Copy link
Member Author

@alpaix some comments:

  1. I think it is not worth doing this (e.g. the caching). For example for code complete the guest always goes to the host. Getting all tasks is not a frequent action so I would go to the host every time.
  2. please be aware that the task are not live objects in the VS Code API and therefore are not === even if they refer to the same task.
  3. I see the problem but I will not expose the internal underlying ID. It is an implementation detail and if I start to expose it I need to keep it. For the tasks that come through a task provider the TaskDefinition can be used as an identifier. This is already speced that way. However for tasks that come from a tasks.json this might not always be the case (if two task use the same exact command line). But I could look into making the TaskDefinition unique in that case as well.
  4. the name of a task is not guaranteed to be unique. Even the tuple [workspace folder, name] must not be unique. So what you need is a way to query a task by task definition.

@alpaix
Copy link

alpaix commented Mar 28, 2018

Agreed on not caching the task entries. This would make the design consistent with the rest of the product.

@alpaix
Copy link

alpaix commented Mar 28, 2018

Regarding 2-4. Will try to come up with a working prototype using the latest API we've got so far.

@alpaix
Copy link

alpaix commented Apr 6, 2018

@dbaeumer Just wanted to clarify how TaskExecution object is supposed to be used. For instance, when executeTask method is called it returns a TaskExecution instance I'd wanted to keep for later. Then after the task terminates it raises onDidEndTask event with another TaskExecution instance attached to it which I cannot match against the first instance I kept as they are not ===. Is this by design?

@dbaeumer
Copy link
Member Author

dbaeumer commented Apr 6, 2018

@alpaix I would accept a bug for this :-). However there will not be any === of tasks for example between to task executions.

@alpaix
Copy link

alpaix commented Apr 9, 2018

Filed a new issue #47465.

@alpaix
Copy link

alpaix commented Apr 9, 2018

Regarding === of tasks we presume combination of task.definition.type with task.name should work as the unique ID of a task in our scenario.

@alpaix
Copy link

alpaix commented Apr 9, 2018

@dbaeumer What's your plan regarding the Task Execution Provider you mentioned earlier? I'm evaluating possible mechanisms of capturing task output in VSCode so I hoped this new provider would help.

@dbaeumer
Copy link
Member Author

dbaeumer commented Apr 9, 2018

@alpaix there will not be a task execution provider API. I am working with @Tyriar to get listening API on the terminal. As a result when a task is executed it should automatically remote if the terminal remotes.

Please note: this will only work for tasks v2.0.0 not for the old task system that uses the Output panel. But I think that is tolerable since the old task runner is deprecated anyways.

@Tyriar
Copy link
Member

Tyriar commented Apr 9, 2018

@alpaix see #46192 (comment) for the current thinking on the API @dbaeumer mentioned.

@egamma egamma added this to the April 2018 milestone Apr 9, 2018
@alpaix
Copy link

alpaix commented Apr 10, 2018

@Tyriar I will work with @IlyaBiryukov on integrating new terminal API with a remote task service on our side.

@alpaix
Copy link

alpaix commented Apr 10, 2018

Thanks for addressing these requests! Appreciate you helping us!

@dbaeumer
Copy link
Member Author

I am closing this item. I added additional API for:

  • get all task executions
  • filters for fetch tasks
  • ===

We still need to find a solution for relating a terminal to a task. This is tracked here: #48032

@dbaeumer
Copy link
Member Author

Please note that the API is still in proposed. I didn't want to make it final due to #48032.

@dbaeumer
Copy link
Member Author

Marking as verified. This got already tested in March.

@dbaeumer dbaeumer added the verified Verification succeeded label Apr 27, 2018
@rickvanprim
Copy link

Would it be possible to get task state and exit code as part of TaskExecution? The first one for so if you cache a TaskExecution you can query if it's still running (instead of watching for onDidTaskEnd() and matching them), and the exit code for knowing whether or not a task succeeded.

@dbaeumer
Copy link
Member Author

dbaeumer commented May 7, 2018

This would make TasKExecution objects live objects which they currently aren't. Adding an exit code to the end event does make sense. Can you please open a separate issue for this?

Why would you prefer querying instead of listening to events?

@rickvanprim
Copy link

Thanks for the response @dbaeumer. Issue created: #49665 . Happy to elaborate if the reasoning inside is insufficient :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-proposal tasks Task system issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

7 participants