atc: behaviour: unpausing archived pipelines fails #5351
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 23, 2020
10 tasks
#5315 Driving this change with tests had some pain points. We decided to work outside-in, starting with an ATC integration test. It was difficult to move from the outer TDD loop to the inner - I was surprised by the number of seemingly-far-flung changes that were required to move from one failure to the next. These included: * adding an entry to the auditor to overcome a panic * modifying the `atc/api/present` package to link the DB entity to the API entity both of these things feel so easy to forget, and it might be nice if they could be described by a change in the same part of the codebase. Signed-off-by: Jamie Klassen <cklassen@pivotal.io> Co-authored-by: Bishoy Youssef <byoussef@pivotal.io> Co-authored-by: Aidan Oldershaw <aoldershaw@pivotal.io>
#5315 - as per #5346 (comment), the ability to archive pipelines is now behind an api flag `enable-archive-pipeline`. Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io> Co-authored-by: James Thomson <jthomson@pivotal.io>
Not sure how that got in there! Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
Specifically for the pipelines/:pipeline_name/archive endpoint Signed-off-by: Bishoy Youssef <byoussef@pivotal.io> Co-authored-by: Taylor Silva <tsilva@pivotal.io>
#5316 Rather than adding a concrete error type to check against, we elected to create an interface for our domain-specific error to satisfy - see: https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully Following the example of trigger-job we decided that 409 Conflict was the most logical http status to use to represent this kind of policy violation. We decided to add the `Conflict` interface to the `db` package because it seems there is no more-appropriate "business logic" or "policy" layer to place this kind of validation error. Since multiple backend members in the web node interact with pipelines directly via the database, it seems justified to place the business logic as close to that integration boundary as possible. Recent painful experiences with observability motivated the choice to treat logging more like a feature, and have unit tests that specifically require it. Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io> Co-authored-by: Jamie Klassen <cklassen@pivotal.io> Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
|
closing this pr as its commits are covered in #5387. |
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Existing Issue
Branched off #5346; it should be merged first.
Fixes #5316.
Changes proposed in this pull request
Contributor Checklist
Reviewer Checklist
New config flags added? Ensure that they are added to the BOSH and Helm packaging; otherwise, ignored for the integration tests (for example, if they are Garden configs that are not displayed in the--helptext).