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.

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

Treat 404 as empty register in AzureBlobStore #108900

Conversation

DaveCTurner
Copy link
Contributor

Closes #108504

@DaveCTurner DaveCTurner added >bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.14.1 v8.15.0 labels May 22, 2024
@DaveCTurner DaveCTurner requested review from tlrx and pxsalehi May 22, 2024 12:42
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Meta label for distributed team label May 22, 2024
@elasticsearchmachine
Copy link
Collaborator

Hi @DaveCTurner, I've created a changelog YAML for you.

@DaveCTurner
Copy link
Contributor Author

Somewhat surprised we hadn't noticed this before, but the stronger check in #107830 catches it now. This commit aligns the implementation with GCS:

final var statusCode = serviceException.getCode();
if (statusCode == RestStatus.NOT_FOUND.getStatus()) {
return OptionalBytesReference.EMPTY;
}

and S3:

if (e.getStatusCode() == 404) {
return OptionalBytesReference.EMPTY;
} else {
throw e;
}

@DaveCTurner DaveCTurner added auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport-and-merge Automatically create backport pull requests and merge when ready labels May 22, 2024
Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this so quickly

@elasticsearchmachine elasticsearchmachine merged commit aa542de into elastic:main May 22, 2024
15 checks passed
@DaveCTurner DaveCTurner deleted the 2024/05/22/AzureBlobStore-get-register-missing branch May 22, 2024 13:37
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 22, 2024
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.14

@DaveCTurner DaveCTurner restored the 2024/05/22/AzureBlobStore-get-register-missing branch June 17, 2024 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport-and-merge Automatically create backport pull requests and merge when ready auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Meta label for distributed team v8.14.1 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] AzureSnapshotRepoTestKitIT testRepositoryAnalysis failing
5 participants