Skip to content

Add --overwrite option to changelog upload - #4075

Open
lcawl wants to merge 14 commits into
mainfrom
changelog-upload-overwrite
Open

lcawl wants to merge 14 commits into
mainfrom
changelog-upload-overwrite

Conversation

@lcawl

@lcawl lcawl commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Addresses #4071

Summary

Add changelog upload option to prevent replacing existing S3 objects.
Every run now also reports how many objects were new versus replaced.

Details

Per #4075 (review) I've changed this PR so overwriting will no longer be the default, but this will be accomplished in a phased way to avoid breaking existing teams' GitHub actions:

  1. This PR: Ship --overwrite while default remains replace (Overwrite still true unless we invert later).
  2. Update actions to pass --overwrite (no-op initially): Support --overwrite option in changelog upload docs-actions#346
  3. Then flip default to Overwrite false. This ensures zero CI gap.

Overwrite option

This PR adds the --overwrite command option and it matches the current behaviour of always replacing (i.e. overwrite defaults to true).
The --skip-etag-check option implies overwrite.

Count new versus replaced on every run

The completion line is now N uploaded (X new, Y replaced), Z skipped, W failed. Refused Puts appear as not overwritten and are not part of uploaded. Per-file logs add (new) or (replace). Classification uses the HeadObject call the uploader already makes.

Example

docs-builder changelog upload \
  --artifact-type changelog \
  --target s3 \
  --s3-bucket-name <bucket> \
  --overwrite

NOTE: While testing in cloud repo, I encountered a local changelog that had multiple repos, so I refined the messaging to be clearer in that case. For example:

Skipped s3://…/153785.yaml: a PR-alias marker already exists at this key with different content. The remote object is a pointer to the canonical changelog for PR 153782, not a full entry. Upload writes this alias because …/153782.yaml lists more than one PR. Leave the existing pointer, or re-run without --no-overwrite to replace it.
Existing remote marker:
link: "153782"

1 PR-alias marker(s) already exist at the destination and were not overwritten.

Generative AI disclosure

  1. Did you use a generative AI (GenAI) tool to assist in creating this contribution?
  • Yes
  • No
  1. If you answered "Yes" to the previous question, please specify the tool(s) and model(s) used (e.g., Google Gemini, OpenAI ChatGPT-4, etc.).

Tool(s) and model(s) used: Cursor Grok 4.6

@github-actions

Copy link
Copy Markdown
Contributor

Docs preview (local build)

Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4075/

Sample changed pages:

@lcawl
lcawl marked this pull request as ready for review September 12, 2026 02:19
@lcawl
lcawl requested a review from a team as a code owner September 12, 2026 02:19
@lcawl
lcawl requested a review from technige September 12, 2026 02:19

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes: --no-overwrite currently uses a non-atomic check-then-put path, so concurrent uploads can still overwrite existing objects. Please enforce no-overwrite with conditional S3 writes and handle precondition failures as not-overwritten conflicts.


What is this? | From workflow: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes: --no-overwrite should treat unchanged inline marker content as skipped, not as a not-overwritten conflict that forces a non-zero result.


What is this? | From workflow: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Comment thread src/services/Elastic.Documentation.Integrations/S3/S3IncrementalUploader.cs Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes.


What is this? | From workflow: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Comment thread src/services/Elastic.Documentation.Integrations/S3/S3IncrementalUploader.cs Outdated
Comment thread src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs Outdated
@lcawl lcawl changed the title Add --no-overwrite option to changelog upload Add --overwrite option to changelog upload Sep 15, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes.


What is this? | From workflow: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Comment thread src/tooling/docs-builder/Commands/ChangelogCommand.cs Outdated
@lcawl
lcawl marked this pull request as draft September 16, 2026 01:08

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No new actionable findings.


What is this? | From workflow: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes.


What is this? | From workflow: PR Review

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Comment thread docs/cli-schema.json
--overwrite
```

```sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[NITPICK] Duplicate example block appears twice

The ### Explicit overwrite section includes the same command snippet twice in succession. Removing the duplicate block will keep the docs concise and avoid suggesting there are two different variants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants