Skip to content

Extract common blob-update logic in S3HttpHandler#138490

Merged
DaveCTurner merged 1 commit intoelastic:mainfrom
DaveCTurner:2025/11/24/s3-handler-extract-if-none-match
Nov 24, 2025
Merged

Extract common blob-update logic in S3HttpHandler#138490
DaveCTurner merged 1 commit intoelastic:mainfrom
DaveCTurner:2025/11/24/s3-handler-extract-if-none-match

Conversation

@DaveCTurner
Copy link
Contributor

The PutObject and CompleteMultipartUpload APIs both implement their
support for the If-None-Match header using identical logic. This
commit extracts the logic into one place to simplify a future change
that adds support for If-Match here too.

The `PutObject` and `CompleteMultipartUpload` APIs both implement their
support for the `If-None-Match` header using identical logic. This
commit extracts the logic into one place to simplify a future change
that adds support for `If-Match` here too.
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v9.3.0 labels Nov 24, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. label Nov 24, 2025
Copy link
Contributor

@joshua-adams-1 joshua-adams-1 left a comment

Choose a reason for hiding this comment

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

LGTM!

} else {
blobs.put(request.path(), blob.v2());
}
final var preconditionFailed = updateBlobContents(exchange, request.path(), blob.v2()) == false;
Copy link
Contributor

Choose a reason for hiding this comment

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

[Nit] You explicitly typed preconditionFailed above and var here. I personally would prefer it to be boolean over var, but it's fine either way!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has to be explicit above because there's no initializer: final var preconditionFailed; doesn't work because it doesn't know what type you mean. Otherwise I'd have used var there too.

(I mean it should be able to work it out based on the first assignment but it doesn't today)

@DaveCTurner DaveCTurner merged commit 7a69799 into elastic:main Nov 24, 2025
34 checks passed
@DaveCTurner DaveCTurner deleted the 2025/11/24/s3-handler-extract-if-none-match branch November 24, 2025 13:09
szybia added a commit to szybia/elasticsearch that referenced this pull request Nov 24, 2025
…-json

* upstream/main: (247 commits)
  Mute org.elasticsearch.xpack.inference.integration.SemanticTextIndexOptionsIT testValidateIndexOptionsWithBasicLicense elastic#138513
  Mute org.elasticsearch.xpack.esql.heap_attack.HeapAttackLookupJoinIT testLookupExplosionBigString elastic#138510
  This shouldn't be zero (elastic#138501)
  sum of empty histogram is now null (elastic#138378)
  Test ES|QL bfloat16 support (elastic#138499)
  Fix exception handling in S3 `compareAndExchangeRegister` (elastic#138488)
  Mute org.elasticsearch.xpack.exponentialhistogram.ExponentialHistogramFieldMapperTests testFormattedDocValues elastic#138504
  Mute org.elasticsearch.ingest.geoip.IngestGeoIpClientYamlTestSuiteIT test {yaml=ingest_geoip/60_ip_location_databases/Test adding, getting, and removing ip location databases} elastic#138502
  ESQL: Refactor HeapAttackIT (elastic#138432)
  [Inference API] Add ElasticInferenceServiceDenseTextEmbeddingsServiceSettings to InferenceNamedWriteablesProvider (elastic#138484)
  Store split indices (elastic#138396)
  ES|QL Update CHUNK to support chunking_settings as optional argument (elastic#138123)
  Extract common blob-update logic in `S3HttpHandler` (elastic#138490)
  Cleanup esql request building api (elastic#138398)
  Round sum and avg in exponential_histogram CSV tests (elastic#138472)
  ESQL: load exponential_histogram total count as double instead of long (elastic#138417)
  [SIMD] Use fixed width native types for better Java interoperability (elastic#138429)
  Do not use Min or Max as Top's surrogate when there is an outputField (elastic#138380)
  ES|QL: Fix generative tests (elastic#138478)
  Mute org.elasticsearch.xpack.inference.integration.AuthorizationTaskExecutorIT testCreatesEisChatCompletion_DoesNotRemoveEndpointWhenNoLongerAuthorized elastic#138480
  ...
afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request Nov 26, 2025
The `PutObject` and `CompleteMultipartUpload` APIs both implement their
support for the `If-None-Match` header using identical logic. This
commit extracts the logic into one place to simplify a future change
that adds support for `If-Match` here too.
ncordon pushed a commit to ncordon/elasticsearch that referenced this pull request Nov 26, 2025
The `PutObject` and `CompleteMultipartUpload` APIs both implement their
support for the `If-None-Match` header using identical logic. This
commit extracts the logic into one place to simplify a future change
that adds support for `If-Match` here too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. >test Issues or PRs that are addressing/adding tests v9.3.0

3 participants