-
Notifications
You must be signed in to change notification settings - Fork 910
Remove the use of S3NativeClient and integrate with S3Client #2976
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
Changes from 4 commits
1e593ae
518a0e8
f501b35
2e4e161
32cc11e
94767f2
9791573
fc31bb8
51cc84e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
|
||
package software.amazon.awssdk.core.internal.http.pipeline.stages; | ||
|
||
import static software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute.SDK_HTTP_EXECUTION_ATTRIBUTES; | ||
import static software.amazon.awssdk.core.internal.http.timers.TimerUtils.resolveTimeoutInMillis; | ||
import static software.amazon.awssdk.http.Header.CONTENT_LENGTH; | ||
|
||
|
@@ -180,15 +181,19 @@ private CompletableFuture<Response<OutputT>> executeHttpRequest(SdkHttpFullReque | |
|
||
MetricCollector httpMetricCollector = MetricUtils.createHttpMetricsCollector(context); | ||
|
||
AsyncExecuteRequest executeRequest = AsyncExecuteRequest.builder() | ||
AsyncExecuteRequest.Builder executeRequestBuilder = AsyncExecuteRequest.builder() | ||
.request(requestWithContentLength) | ||
.requestContentPublisher(requestProvider) | ||
.responseHandler(wrappedResponseHandler) | ||
.fullDuplex(isFullDuplex(context.executionAttributes())) | ||
.metricCollector(httpMetricCollector) | ||
.build(); | ||
.metricCollector(httpMetricCollector); | ||
if (context.executionAttributes().getAttribute(SDK_HTTP_EXECUTION_ATTRIBUTES) != null) { | ||
executeRequestBuilder.httpExecutionAttributes( | ||
context.executionAttributes() | ||
.getAttribute(SDK_HTTP_EXECUTION_ATTRIBUTES)); | ||
Comment on lines
+191
to
+193
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if we just copied all There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, most of the execution attributes are used in the SDK core and not relevant to the HTTP request. |
||
} | ||
|
||
CompletableFuture<Void> httpClientFuture = doExecuteHttpRequest(context, executeRequest); | ||
CompletableFuture<Void> httpClientFuture = doExecuteHttpRequest(context, executeRequestBuilder.build()); | ||
|
||
TimeoutTracker timeoutTracker = setupAttemptTimer(responseFuture, context); | ||
context.apiCallAttemptTimeoutTracker(timeoutTracker); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"). | ||
* You may not use this file except in compliance with the License. | ||
* A copy of the License is located at | ||
* | ||
* http://aws.amazon.com/apache2.0 | ||
* | ||
* or in the "license" file accompanying this file. This file is distributed | ||
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
* express or implied. See the License for the specific language governing | ||
* permissions and limitations under the License. | ||
*/ | ||
|
||
package software.amazon.awssdk.http; | ||
|
||
import software.amazon.awssdk.annotations.SdkPublicApi; | ||
import software.amazon.awssdk.http.async.AsyncExecuteRequest; | ||
import software.amazon.awssdk.utils.AttributeMap; | ||
|
||
/** | ||
* An attribute attached to a particular HTTP request execution, stored in {@link SdkHttpExecutionAttributes}. It can be | ||
* configured on an {@link AsyncExecuteRequest} via | ||
* {@link AsyncExecuteRequest.Builder#putHttpExecutionAttribute(SdkHttpExecutionAttribute, | ||
* Object)} | ||
* | ||
* @param <T> The type of data associated with this attribute. | ||
*/ | ||
@SdkPublicApi | ||
public abstract class SdkHttpExecutionAttribute<T> extends AttributeMap.Key<T> { | ||
|
||
protected SdkHttpExecutionAttribute(Class<T> valueType) { | ||
super(valueType); | ||
} | ||
|
||
protected SdkHttpExecutionAttribute(UnsafeValueType unsafeValueType) { | ||
super(unsafeValueType); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"). | ||
* You may not use this file except in compliance with the License. | ||
* A copy of the License is located at | ||
* | ||
* http://aws.amazon.com/apache2.0 | ||
* | ||
* or in the "license" file accompanying this file. This file is distributed | ||
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
* express or implied. See the License for the specific language governing | ||
* permissions and limitations under the License. | ||
*/ | ||
|
||
package software.amazon.awssdk.http; | ||
|
||
import java.util.Map; | ||
import java.util.Objects; | ||
import software.amazon.awssdk.annotations.SdkPublicApi; | ||
import software.amazon.awssdk.http.async.AsyncExecuteRequest; | ||
import software.amazon.awssdk.utils.AttributeMap; | ||
import software.amazon.awssdk.utils.Validate; | ||
import software.amazon.awssdk.utils.builder.CopyableBuilder; | ||
import software.amazon.awssdk.utils.builder.ToCopyableBuilder; | ||
|
||
/** | ||
* An immutable collection of {@link SdkHttpExecutionAttribute}s that can be configured on an {@link AsyncExecuteRequest} via | ||
* {@link AsyncExecuteRequest.Builder#httpExecutionAttributes(SdkHttpExecutionAttributes)} | ||
*/ | ||
@SdkPublicApi | ||
public final class SdkHttpExecutionAttributes implements ToCopyableBuilder<SdkHttpExecutionAttributes.Builder, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I added |
||
SdkHttpExecutionAttributes> { | ||
private final AttributeMap attributes; | ||
|
||
private SdkHttpExecutionAttributes(Builder builder) { | ||
this.attributes = builder.sdkHttpExecutionAttributes.build(); | ||
} | ||
|
||
/** | ||
* Retrieve the current value of the provided attribute in this collection of attributes. This will return null if the value | ||
* is not set. | ||
*/ | ||
public <T> T attribute(SdkHttpExecutionAttribute<T> attribute) { | ||
zoewangg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return attributes.get(attribute); | ||
} | ||
|
||
public static Builder builder() { | ||
return new Builder(); | ||
} | ||
|
||
@Override | ||
public Builder toBuilder() { | ||
return new Builder(attributes); | ||
} | ||
|
||
public SdkHttpExecutionAttributes copy() { | ||
zoewangg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return toBuilder().build(); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
|
||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
|
||
SdkHttpExecutionAttributes that = (SdkHttpExecutionAttributes) o; | ||
|
||
return Objects.equals(attributes, that.attributes); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return attributes.hashCode(); | ||
} | ||
|
||
public static final class Builder implements CopyableBuilder<SdkHttpExecutionAttributes.Builder, SdkHttpExecutionAttributes> { | ||
private AttributeMap.Builder sdkHttpExecutionAttributes = AttributeMap.builder(); | ||
|
||
private Builder(AttributeMap attributes) { | ||
sdkHttpExecutionAttributes = attributes.toBuilder(); | ||
} | ||
|
||
private Builder() { | ||
} | ||
|
||
/** | ||
* Add a mapping between the provided key and value. | ||
*/ | ||
public <T> SdkHttpExecutionAttributes.Builder put(SdkHttpExecutionAttribute<T> key, T value) { | ||
Validate.notNull(key, "Key to set must not be null."); | ||
sdkHttpExecutionAttributes.put(key, value); | ||
return this; | ||
} | ||
|
||
/** | ||
* Adds all the attributes from the map provided. | ||
*/ | ||
public SdkHttpExecutionAttributes.Builder putAll(Map<? extends SdkHttpExecutionAttribute<?>, ?> attributes) { | ||
sdkHttpExecutionAttributes.putAll(attributes); | ||
return this; | ||
} | ||
|
||
@Override | ||
public SdkHttpExecutionAttributes build() { | ||
return new SdkHttpExecutionAttributes(this); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,8 +17,11 @@ | |
|
||
import java.util.Optional; | ||
import software.amazon.awssdk.annotations.SdkPublicApi; | ||
import software.amazon.awssdk.http.SdkHttpExecutionAttribute; | ||
import software.amazon.awssdk.http.SdkHttpExecutionAttributes; | ||
import software.amazon.awssdk.http.SdkHttpRequest; | ||
import software.amazon.awssdk.metrics.MetricCollector; | ||
import software.amazon.awssdk.utils.Validate; | ||
|
||
/** | ||
* Request object containing the parameters necessary to make an asynchronous HTTP request. | ||
|
@@ -32,13 +35,15 @@ public final class AsyncExecuteRequest { | |
private final SdkAsyncHttpResponseHandler responseHandler; | ||
private final MetricCollector metricCollector; | ||
private final boolean isFullDuplex; | ||
private final SdkHttpExecutionAttributes sdkHttpExecutionAttributes; | ||
|
||
private AsyncExecuteRequest(BuilderImpl builder) { | ||
this.request = builder.request; | ||
this.requestContentPublisher = builder.requestContentPublisher; | ||
this.responseHandler = builder.responseHandler; | ||
this.metricCollector = builder.metricCollector; | ||
this.isFullDuplex = builder.isFullDuplex; | ||
this.sdkHttpExecutionAttributes = builder.executionAttributesBuilder.build(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be referencing a builder or an already-built object? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean? Is there any benefit of referencing an already-built object? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems unusual to me that we're calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, executionAttributes is immutable, so we have to keep a |
||
} | ||
|
||
/** | ||
|
@@ -76,6 +81,13 @@ public boolean fullDuplex() { | |
return isFullDuplex; | ||
} | ||
|
||
/** | ||
* @return the SDK HTTP execution attributes associated with this request | ||
*/ | ||
public SdkHttpExecutionAttributes httpExecutionAttributes() { | ||
return sdkHttpExecutionAttributes; | ||
} | ||
|
||
public static Builder builder() { | ||
return new BuilderImpl(); | ||
} | ||
|
@@ -125,6 +137,25 @@ public interface Builder { | |
*/ | ||
Builder fullDuplex(boolean fullDuplex); | ||
|
||
/** | ||
* Put an HTTP execution attribute into to the collection of HTTP execution attributes for this request | ||
* | ||
* @param attribute The execution attribute object | ||
* @param value The value of the execution attribute. | ||
*/ | ||
<T> Builder putHttpExecutionAttribute(SdkHttpExecutionAttribute<T> attribute, T value); | ||
|
||
/** | ||
* Sets the additional HTTP execution attributes collection for this request. | ||
* <p> | ||
* This will override the attributes configured through | ||
* {@link #putHttpExecutionAttribute(SdkHttpExecutionAttribute, Object)} | ||
* | ||
* @param executionAttributes Execution attributes map for this request. | ||
* @return This object for method chaining. | ||
*/ | ||
Builder httpExecutionAttributes(SdkHttpExecutionAttributes executionAttributes); | ||
|
||
AsyncExecuteRequest build(); | ||
} | ||
|
||
|
@@ -134,6 +165,7 @@ private static class BuilderImpl implements Builder { | |
private SdkAsyncHttpResponseHandler responseHandler; | ||
private MetricCollector metricCollector; | ||
private boolean isFullDuplex; | ||
private SdkHttpExecutionAttributes.Builder executionAttributesBuilder = SdkHttpExecutionAttributes.builder(); | ||
|
||
@Override | ||
public Builder request(SdkHttpRequest request) { | ||
|
@@ -165,6 +197,20 @@ public Builder fullDuplex(boolean fullDuplex) { | |
return this; | ||
} | ||
|
||
@Override | ||
public <T> Builder putHttpExecutionAttribute(SdkHttpExecutionAttribute<T> attribute, T value) { | ||
this.executionAttributesBuilder.put(attribute, value); | ||
return this; | ||
} | ||
|
||
@Override | ||
public Builder httpExecutionAttributes(SdkHttpExecutionAttributes executionAttributes) { | ||
Validate.paramNotNull(executionAttributes, "executionAttributes"); | ||
this.executionAttributesBuilder = executionAttributes.toBuilder(); | ||
return this; | ||
} | ||
|
||
|
||
@Override | ||
public AsyncExecuteRequest build() { | ||
return new AsyncExecuteRequest(this); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"). | ||
* You may not use this file except in compliance with the License. | ||
* A copy of the License is located at | ||
* | ||
* http://aws.amazon.com/apache2.0 | ||
* | ||
* or in the "license" file accompanying this file. This file is distributed | ||
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
* express or implied. See the License for the specific language governing | ||
* permissions and limitations under the License. | ||
*/ | ||
|
||
package software.amazon.awssdk.http; | ||
|
||
|
||
import static org.assertj.core.api.AssertionsForClassTypes.assertThat; | ||
|
||
import nl.jqno.equalsverifier.EqualsVerifier; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class SdkHttpExecutionAttributesTest { | ||
|
||
@Test | ||
void equalsAndHashcode() { | ||
EqualsVerifier.forClass(SdkHttpExecutionAttributes.class) | ||
.withNonnullFields("attributes") | ||
.verify(); | ||
} | ||
|
||
@Test | ||
void getAttribute_shouldReturnCorrectValue() { | ||
SdkHttpExecutionAttributes attributes = SdkHttpExecutionAttributes.builder() | ||
.put(TestExecutionAttribute.TEST_KEY_FOO, "test") | ||
.build(); | ||
assertThat(attributes.attribute(TestExecutionAttribute.TEST_KEY_FOO)).isEqualTo("test"); | ||
} | ||
|
||
private static final class TestExecutionAttribute<T> extends SdkHttpExecutionAttribute<T> { | ||
|
||
private static final TestExecutionAttribute<String> TEST_KEY_FOO = new TestExecutionAttribute<>(String.class); | ||
|
||
private TestExecutionAttribute(Class valueType) { | ||
super(valueType); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a variant of
getAttribute()
that returns anOptional
, so we don't have to do multiple lookups? It's a hyper-nit in terms of performance, but it would also help readability in places like this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it'd be nice to have another method returning optional but since overload is not possible, I'd be hesitant to introduce another method with a different name unless we have strong reasons.