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.

Sign up for GitHub

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

Jump to bottom

Clarify --include-retries docs and fixes #1341

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+10 −5
Open

Clarify --include-retries docs and fixes #1341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6d2d7d
Minor docs fixes
plaindocs Nov 27, 2020
8d4730a
Update clicommand/artifact_download.go
plaindocs Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions clicommand/artifact_download.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,22 @@ Description:

Example:

$ buildkite-agent artifact download "pkg/*.tar.gz" . --build xxx
$ buildkite-agent artifact download --build xxx "pkg/*.tar.gz" .

This will search across all the artifacts for the build with files that match that part.
The first argument is the search query, and the second argument is the download destination.

If you're trying to download a specific file, and there are multiple artifacts from different
jobs, you can target the particular job you want to download the artifact from:

$ buildkite-agent artifact download "pkg/*.tar.gz" . --step "tests" --build xxx
$ buildkite-agent artifact download --step "tests" --build xxx "pkg/*.tar.gz" .

You can also use the step's jobs id (provided by the environment variable $BUILDKITE_JOB_ID)`
You can also use the step's job id (provided by the environment variable $BUILDKITE_JOB_ID)

By default, only artifacts from the most recent job in a chain of retried jobs are downloaded.
To include artifacts from previous retried jobs, use the "--include-retried-jobs" flag:
Comment on lines +37 to +38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this makes it clearer? “Retried” really feels like it does double duty here:

Suggested change
By default, only artifacts from the most recent job in a chain of retried jobs are downloaded.
To include artifacts from previous retried jobs, use the "--include-retried-jobs" flag:
By default, only artifacts from the last job in a chain of retried jobs are downloaded.
To include artifacts from superseded jobs, use the "--include-retried-jobs" flag:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm less keen on superseded, because if I read that I'd be wondering if it meant something specific in this context. We could just use "from previous jobs"

Huh, that was unintentional:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m just not sure “previous jobs” conveys “previous tries of this step” though, it seems ambiguous whether this means “prior jobs in the build” or something else

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree. How about

Suggested change
To include artifacts from previous retried jobs, use the "--include-retried-jobs" flag:
To include artifacts also from all retried jobs belonging to this step, use the "--include-retried-jobs" flag:

"Also", to make clear that it is as well as the last one. What do you reckon?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @ticky

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure this solves for the ambiguity in “retried,” and do we otherwise expose the step/job dichotomy to users?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll ponder and reword tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We expose step just a couple of lines above, on line 35. But if we're not specifying a job ID, we're just using the artifact path, right, so this flag is just saying "get all the stuff, not just the most recent stuff", so mentioning the step isn't helpful. This might be better:

Suggested change
To include artifacts from previous retried jobs, use the "--include-retried-jobs" flag:
To include matching artifacts also from previous retried jobs as well as the most recent, use the "--include-retried-jobs" flag:


$ buildkite-agent artifact download --include-retried-jobs "pkg/*.tar.gz" .`

type ArtifactDownloadConfig struct {
Query string `cli:"arg:0" label:"artifact search query" validate:"required"`
Expand Down Expand Up @@ -73,7 +78,7 @@ var ArtifactDownloadCommand = cli.Command{
cli.BoolFlag{
Name: "include-retried-jobs",
EnvVar: "BUILDKITE_AGENT_INCLUDE_RETRIED_JOBS",
Usage: "Include artifacts from retried jobs in the search",
Usage: "Include artifacts from previous retried jobs in the search",
},

// API Flags
Expand Down
2 changes: 1 addition & 1 deletion clicommand/artifact_upload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Description:
You can specify an alternate destination on Amazon S3, Google Cloud Storage
or Artifactory as per the examples below. This may be specified in the
'destination' argument, or in the 'BUILDKITE_ARTIFACT_UPLOAD_DESTINATION'
environment variable. Otherwise, artifacts are uploaded to a
environment variable. Otherwise, artifacts are uploaded to a
Buildkite-managed Amazon S3 bucket.

Example:
Expand Down