Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Support updating existing tasks #37

Closed
JeanMertz opened this issue Jul 20, 2019 · 4 comments
Closed

Support updating existing tasks #37

JeanMertz opened this issue Jul 20, 2019 · 4 comments
Labels
enhancement New feature or request

Comments

@JeanMertz
Copy link
Contributor

Right now the API only supports creating tasks. In a continuous deployment setup of Automaat, you'd want to be able to (for example) have your tasks defined as YAML in Git, and then deployed to your Automaat instance using your CI pipeline once a task change is pushed to your base branch.

I think it makes sense to use the OnConflict object that already exists for creating/updating global variables:

enum OnConflict {
ABORT
UPDATE
}

For tasks, there are some considerations, that will probably require an extra OnConflict directive.

Here are the thoughts I currently have:

  • The same conflict directives (ABORT, UPDATE) apply to tasks.
  • The default if unset is the same as global variables (and the same as the current behaviour of tasks), which is to abort.
  • Updating a task means that the following should happen:
    • The task is matched using the task name (which is unique).
    • Any mutable task properties are updated (everything except its ID and name*)
    • All steps matching the task step names are updated.
    • All steps with new task step names are added.
    • All task steps in the database that aren't part of this mutation are removed.
    • Same thing happens for variables.
  • In addition to the existing two directives, I think it makes sense to add an ARCHIVE directive, that works specifically for tasks (for now).
    • It's purpose is to keep a record of previous tasks and their configurations.
    • When this directive is given, an existing task with the same name has its archived flag set.
    • This task is no longer returned in API responses.
    • It is also no longer invalid to have two or more tasks with the same name, as long as only one is non-archived.
    • Other than that, this directive creates as new task, as if it didn't exist before.
    • The upside of this is that we can keep the job -> task connections for archived tasks.

Another solution to the above point would be to simply remove the relationship between jobs and tasks. However, that would mean we can no longer show a history of jobs for any given task. The reference is already weak, so tasks are allowed to be removed without removing the jobs they created, this just means you get a null value back for the reference from a job back to a deleted task.

One more point to cover when we allow updating tasks is that it can no longer be assumed that the configuration of a job matches that of a task (meaning, a job could have been triggered when a task had different variables or steps configured). This is fine, but I think it makes sense to at least make sure there are created_at/updated_at fields available, so that we can easily detect if a task was changed after a job was created, which could be used in the future to signal that the representation of a job might not match that of an existing task.

All of this could also be avoided by making tasks immutable, and simply creating new ones and archiving old ones for every change you make. But I think that's a bit too extreme of a solution in this case (something like event sourcing would help here, but that's way out of scope).

* because we are re-using the createTask mutation, we can't ask for a task ID because none might exist yet, so we also can't rename the task. We might also supply an updateTaskName mutation that takes and ID, so that you can change the name as well, but I'll leave that out of the scope for now. You can always rename a task using the database itself, until we find a nice way to use this API.

@JeanMertz
Copy link
Contributor Author

JeanMertz commented Jul 20, 2019

A few more notes:

  • I think we can delay the introduction of ARCHIVE for now, as long as updating works.
  • updateTaskName would be nice, but can also wait for a bit.
  • We should already add the created_at/updated_at as part of this change.
  • We should also take into account the variable advertisements (3d45e61) and apply the same logic as steps/variables (remove if unset in update, update otherwise).

@JeanMertz
Copy link
Contributor Author

Thinking on this some more, I think the semantics on this could be (for starters) as straight-forward as this:

  • When the conflict directive is set to UPDATE
    • Delete all steps related to a task (which will cascade to also delete variable_advertisements)
    • Delete all variables related to a task
    • Update the task description and labels, if they changed
    • Create new task steps and variables
  • When the conflict directive is set to ABORT
    • Create the task if its name doesn't exist yet, otherwise abort

This should cover all use-cases, except for task renaming, which will have to be its own mutation request, for now, and can be tackled later.

@JeanMertz
Copy link
Contributor Author

Actually, one problem I just realised with the above approach is that this would mean any CI-based deployment pipeline would recreate the steps/variables for all tasks on every deploy.

Technically, that's not a problem, but it does feel a bit draconian. 🤔 💭

Of course, we could consider that out of scope, and provide some helpful pointers on how to only trigger an update for tasks that have actually changed (using your VCS' history).

@JeanMertz JeanMertz added the enhancement New feature or request label Aug 25, 2019
@JeanMertz
Copy link
Contributor Author

This was solved in 9bc4164.

Here's the description from the commit:


This change introduces the possibility to update tasks.

The implementation is similar to global variables introduced in
3932d55:

A new onConflict directive can be passed along with the task details.
It can be either UPDATE or ABORT, it defaults to ABORT if unset.

If set to ABORT, the behavior will be similar to the existing
behavior, and an error is returned if a similarly named task already
exists.

If set to UPDATE, the following semantics apply:

  • If a task with a similar name does not yet exist, a new task is
    created (this also means that task names currently can't be renamed
    via the API, but they can be modified in the database directly).

  • If a similarly named task name does exist:

    • its properties (description, labels) are updated,
    • its steps are matched using their names (you can update step
      names, but it will remove the old step, and create a new step,
      instead of updating the existing one),
    • step properties (description, processor, position,
      advertisedVariableKey) are updated,
    • missing steps are removed,
    • new steps are added,
    • the same rules apply to task variables, you can update their
      description, selectionConstraint, defaultValue and exampleValue.

There is no support in the client(s) yet for creating or updating tasks,
but using the exposed GraphQL API playground, you can achieve the same
results.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant