go-concourse: Add Archive to go-concourse #5365
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
10 tasks
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>
aoldershaw
force-pushed
the
issue/5319
branch
2 times, most recently
from
1a76e85
to
25b6fb7
Compare
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>
Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io> Co-authored-by: Jamie Klassen <cklassen@pivotal.io>
#5318 Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
#5319 Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io> Co-authored-by: James Thomson <jthomson@pivotal.io>
#5319 Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io> Co-authored-by: James Thomson <jthomson@pivotal.io>
#5319 Now that we have an `ArchivePipeline` method in go-concourse, we can use that in the integration tests for archiving pipelines (rather than caling the endpoint directly). Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io> Co-authored-by: James Thomson <jthomson@pivotal.io>
|
closing this pr as its commits are covered in #5387. |
9 tasks
aoldershaw
added a commit
that referenced
this pull request
Jun 7, 2020
#5365 * Banner is a slightly darker gray than the BG * Job preview is an empty placeholder (like when ListAllJobs is disabled) * No status icon/text * No pause toggle Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
aoldershaw
added a commit
that referenced
this pull request
Jun 16, 2020
#5365 * Banner is a slightly darker gray than the BG * Job preview is an empty placeholder (like when ListAllJobs is disabled) * No status icon/text * No pause toggle Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io> Co-authored-by: Aidan Oldershaw <aoldershaw@pivotal.io>
aoldershaw
added a commit
that referenced
this pull request
Jun 24, 2020
#5365 * Banner is a slightly darker gray than the BG * Job preview is an empty placeholder (like when ListAllJobs is disabled) * No status icon/text * No pause toggle Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io> Co-authored-by: Aidan Oldershaw <aoldershaw@pivotal.io>
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 of #5359 which should be merged first.
Fixes #5319
Why do we need this PR?
Continuing the road to Archiving pipelines, this PR adds the archive functionality to go-concourse.
Contributor Checklist
Integration tests (if applicable)Updated documentation (located at https://github.com/concourse/docs)Updated release notes (located at https://github.com/concourse/concourse/tree/master/release-notes)Reviewer Checklist
and Helm packaging; otherwise, ignored for the integration tests (for example, if they are Garden configs that are not displayed in the
--helptext).