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

Remove Legacy Enums #3235

Merged
merged 2 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion generator/ServiceClientGeneratorLib/Customizations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1554,7 +1554,7 @@ public Override()
/// any code that would be valid within the Amazon[ServiceName]Client
/// class for the ServiceName where the override is being made. For
/// example, a valid condition is:
/// "if(this.Config.RetryMode == RequestRetryMode.Legacy)"
/// "if(this.Config.RetryMode == RequestRetryMode.Standard)"
/// </summary>
public string Condition { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ public interface IDefaultConfigurationController
{
/// <summary>
/// Fully loads and populates <see cref="DefaultConfigurationModel"/> by parsing the
/// sdk-default-configurations.json file from disk. Also adds the 'Legacy'
/// <see cref="DefaultConfigurationMode"/> with the correct defaults.
/// sdk-default-configurations.json file from disk.
/// </summary>
/// <param name="repositoryRootDirectoryPath">
/// Full path to the root directory, ie contains the 'sdk' directory.
Expand Down Expand Up @@ -52,33 +51,11 @@ public DefaultConfigurationModel LoadDefaultConfiguration(string repositoryRootD
}

var parsedModel = _defaultConfigurationParser.Parse(json);

EnrichLegacyMode(parsedModel);


var validModes = parsedModel.Modes.Where(m => m.Name.ToPascalCase() != "Legacy").ToList();
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we should add this part, maybe we will need to add a new mode named Legacy in the future for any reason.

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 think that's unlikely, Legacy is already one of the modes (https://docs.aws.amazon.com/sdkref/latest/guide/feature-smart-config-defaults.html), if a new one is added in the future we'd have to pick a new name.

Either way, it'd require us to update generator/ServiceClientGeneratorLib/DefaultConfiguration/DefaultConfigurationModel.cs as well.

parsedModel.Modes = validModes;

return parsedModel;
}

private void EnrichLegacyMode(DefaultConfigurationModel parsedModel)
{
var legacyMode =
parsedModel
.Modes
.FirstOrDefault(x => string.Equals(x.Name, "Legacy", StringComparison.OrdinalIgnoreCase));

if (legacyMode == null)
throw new Exception(
"Did not find required Default Configuration mode 'Legacy'. " +
$"Found: {string.Join(",", parsedModel.Modes.Select(x => x.Name))}");

legacyMode.RetryMode = RequestRetryMode.Legacy;
legacyMode.S3UsEast1RegionalEndpoint = S3UsEast1RegionalEndpointValue.Legacy;
legacyMode.StsRegionalEndpoints = StsRegionalEndpointsValue.Legacy;
// default to null for timeouts - this preserves the ServiceConfig
// behavior of defaulting configurable timeouts to null
legacyMode.TimeToFirstByteTimeout = null;
legacyMode.ConnectTimeout = null;
legacyMode.HttpRequestTimeout = null;
legacyMode.TlsNegotiationTimeout = null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class DefaultConfigurationModel
public class DefaultConfigurationMode
{
/// <summary>
/// Identifies a specific configuration mode. Example legacy, mobile, cross-region, etc
/// Identifies a specific configuration mode. Example mobile, cross-region, etc
/// </summary>
public string Name { get; set; }
/// <summary>
Expand Down Expand Up @@ -79,10 +79,6 @@ public class DefaultConfigurationMode
/// <remarks>This is a copy of Amazon.Runtime.RequestRetryMode</remarks>
public enum RequestRetryMode
{
/// <summary>
/// Legacy request retry strategy.
/// </summary>
Legacy,
/// <summary>
/// Standardized request retry strategy that is consistent across all SDKs.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@ namespace ServiceClientGeneratorTests
public class DefaultConfigurationControllerTests
{
/// <summary>
/// Controller should find the 'Legacy' Default Configuration Mode
/// and explicitly set timeouts.
/// Controller should ignore the 'Legacy' Default Configuration Mode.
/// </summary>
[Fact]
public void ControllerEnrichesLegacyDefaultConfigurationMode()
public void ControllerIgnoresLegacyDefaultConfigurationMode()
{
var fakeJson = "fakeJson";
var fakeDocumentation = "fakeDocumentation";
Expand Down Expand Up @@ -51,10 +50,7 @@ public void ControllerEnrichesLegacyDefaultConfigurationMode()
.Modes
.FirstOrDefault(x => x.Name == "Legacy");

legacyMode.ShouldNotBeNull();
legacyMode.Documentation.ShouldEqual(fakeDocumentation);
// Controller should have modified timeout
legacyMode.TimeToFirstByteTimeout.ShouldBeGreaterThan(TimeSpan.Zero);
legacyMode.ShouldBeNull();
}
}
}
11 changes: 0 additions & 11 deletions generator/ServiceModels/dynamodb/dynamodb-.customizations.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,6 @@
"noArgOverloads": [
"ListTables"
],
"runtimePipelineOverride":{
"overrides":[
{
"condition":"this.Config.RetryMode == RequestRetryMode.Legacy",
"operation":"replace",
"newType":"Amazon.Runtime.Internal.RetryHandler",
"targetType":"Amazon.Runtime.Internal.RetryHandler",
"constructorInput":"new Amazon.DynamoDBv2.Internal.DynamoDBRetryPolicy(this.Config)"
}
]
},
"emitIsSetProperties":{
"AttributeValue" : [
"BOOL",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,7 @@
{
"runtimePipelineOverride":{
"overrides":[
{
"condition":"this.Config.RetryMode == RequestRetryMode.Legacy",
"operation":"replace",
"newType":"Amazon.Runtime.Internal.RetryHandler",
"targetType":"Amazon.Runtime.Internal.RetryHandler",
"constructorInput":"new Amazon.DynamoDBv2.Internal.DynamoDBRetryPolicy(this.Config)"
}
]
},
"shapeSubstitutions": {
"Stream": {
"renameShape": "StreamSummary"
}
}
{
"shapeSubstitutions": {
"Stream": {
"renameShape": "StreamSummary"
}
}
}
11 changes: 2 additions & 9 deletions generator/ServiceModels/ec2/ec2.customizations.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,14 @@
"newType": "Amazon.EC2.Internal.AmazonEC2ResponseHandler",
"targetType": "Amazon.Runtime.Internal.Unmarshaller"
},
{
"condition":"this.Config.RetryMode == RequestRetryMode.Legacy",
"operation":"replace",
"newType":"Amazon.Runtime.Internal.RetryHandler",
"targetType":"Amazon.Runtime.Internal.RetryHandler",
"constructorInput":"new Amazon.EC2.Internal.EC2RetryPolicy(this.Config)"
},
{
{
"condition":"this.Config.RetryMode == RequestRetryMode.Standard",
"operation":"replace",
"newType":"Amazon.Runtime.Internal.RetryHandler",
"targetType":"Amazon.Runtime.Internal.RetryHandler",
"constructorInput":"new Amazon.EC2.Internal.EC2StandardRetryPolicy(this.Config)"
},
{
{
"condition":"this.Config.RetryMode == RequestRetryMode.Adaptive",
"operation":"replace",
"newType":"Amazon.Runtime.Internal.RetryHandler",
Expand Down
7 changes: 0 additions & 7 deletions generator/ServiceModels/s3/s3.customizations.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,6 @@
{ "operation": "addAfter", "newType": "Amazon.S3.Internal.AmazonS3RedirectHandler", "targetType": "Amazon.Runtime.Internal.Unmarshaller" },
{ "operation": "addBefore", "newType": "Amazon.S3.Internal.S3Express.S3ExpressPreSigner", "targetType": "Amazon.Runtime.Internal.Signer" },
{ "operation": "addAfter", "newType": "Amazon.S3.Internal.AmazonS3PostMarshallHandler", "targetType": "Amazon.Runtime.Internal.EndpointResolver" },
{
"condition":"this.Config.RetryMode == RequestRetryMode.Legacy",
"operation": "replace",
"newType": "Amazon.Runtime.Internal.RetryHandler",
"targetType": "Amazon.Runtime.Internal.RetryHandler",
"constructorInput": "new Amazon.S3.Internal.AmazonS3RetryPolicy(this.Config)"
},
{
"condition":"this.Config.RetryMode == RequestRetryMode.Standard",
"operation": "replace",
Expand Down
7 changes: 0 additions & 7 deletions generator/ServiceModels/sts/sts.customizations.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
{
"runtimePipelineOverride": {
"overrides": [
{
"condition":"this.Config.RetryMode == RequestRetryMode.Legacy",
"operation": "replace",
"newType": "Amazon.Runtime.Internal.RetryHandler",
"targetType": "Amazon.Runtime.Internal.RetryHandler",
"constructorInput": "new SecurityTokenServiceRetryPolicy(this.Config)"
},
{
"condition":"this.Config.RetryMode == RequestRetryMode.Standard",
"operation": "replace",
Expand Down
3 changes: 0 additions & 3 deletions sdk/src/Core/Amazon.Runtime/AmazonServiceClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,6 @@ private void BuildRuntimePipeline()
case RequestRetryMode.Standard:
retryPolicy = new StandardRetryPolicy(this.Config);
break;
case RequestRetryMode.Legacy:
retryPolicy = new DefaultRetryPolicy(this.Config);
break;
default:
throw new InvalidOperationException("Unknown retry mode");
}
Expand Down
45 changes: 2 additions & 43 deletions sdk/src/Core/Amazon.Runtime/ClientConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ public abstract partial class ClientConfig : IClientConfig
private RequestRetryMode? retryMode = null;
private int? maxRetries = null;
private const int MaxRetriesDefault = 2;
private const int MaxRetriesLegacyDefault = 4;
private const long DefaultMinCompressionSizeBytes = 10240;
private bool didProcessServiceURL = false;
private IAWSTokenProvider _awsTokenProvider = new DefaultAWSTokenProviderChain();
Expand Down Expand Up @@ -462,8 +461,7 @@ public string ServiceId
}
/// <summary>
/// Returns the flag indicating how many retry HTTP requests an SDK should
/// make for a single SDK operation invocation before giving up. This flag will
/// return 4 when the RetryMode is set to "Legacy" which is the default. For
/// make for a single SDK operation invocation before giving up. For
/// RetryMode values of "Standard" or "Adaptive" this flag will return 2. In
/// addition to the values returned that are dependent on the RetryMode, the
/// value can be set to a specific value by using the AWS_MAX_ATTEMPTS environment
Expand All @@ -480,13 +478,6 @@ public int MaxErrorRetry
{
if (!this.maxRetries.HasValue)
{
//For legacy mode there was no MaxAttempts shared config or
//environment variables so use the legacy default value.
if (RetryMode == RequestRetryMode.Legacy)
{
return MaxRetriesLegacyDefault;
}

//For standard and adaptive modes first check the environment variables
//and shared config for a value. Otherwise default to the new default value.
//In the shared config or environment variable MaxAttempts is the total number
Expand Down Expand Up @@ -720,33 +711,6 @@ protected ClientConfig(IDefaultConfigurationProvider defaultConfigurationProvide
Initialize();
}

public ClientConfig() : this(new LegacyOnlyDefaultConfigurationProvider())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This constructor was not used by the service clients; I did have to modify one test to pass in a mocked provider.

{
this.defaultConfigurationBackingField = _defaultConfigurationProvider.GetDefaultConfiguration(null, null);
this.defaultConfigurationMode = this.defaultConfigurationBackingField.Name;
}

/// <summary>
/// Specialized <see cref="IDefaultConfigurationProvider"/> that is only meant to provide backwards
/// compatibility for the obsolete <see cref="ClientConfig"/> constructor.
/// </summary>
private class LegacyOnlyDefaultConfigurationProvider : IDefaultConfigurationProvider
{
public IDefaultConfiguration GetDefaultConfiguration(RegionEndpoint clientRegion, DefaultConfigurationMode? requestedConfigurationMode = null)
{
if (requestedConfigurationMode.HasValue &&
requestedConfigurationMode.Value != Runtime.DefaultConfigurationMode.Legacy)
throw new AmazonClientException($"This ClientConfig only supports {Runtime.DefaultConfigurationMode.Legacy}");

return new DefaultConfiguration
{
Name = Runtime.DefaultConfigurationMode.Legacy,
RetryMode = RequestRetryMode.Legacy,
S3UsEast1RegionalEndpoint = S3UsEast1RegionalEndpointValue.Legacy,
StsRegionalEndpoints = StsRegionalEndpointsValue.Legacy
};
}
}
#endregion

protected virtual void Initialize()
Expand Down Expand Up @@ -801,10 +765,6 @@ public TimeSpan? Timeout
/// </summary>
internal CancellationToken BuildDefaultCancellationToken()
{
// legacy mode never had a working cancellation token, so keep it to default()
if (DefaultConfiguration.Name == Runtime.DefaultConfigurationMode.Legacy)
return default(CancellationToken);

// TimeToFirstByteTimeout is not a perfect match with HttpWebRequest/HttpClient.Timeout. However, given
// that both are configured to only use Timeout until the Response Headers are downloaded, this value
// provides a reasonable default value.
Expand All @@ -816,7 +776,6 @@ internal CancellationToken BuildDefaultCancellationToken()
}
#endif


/// <summary>
/// Configures the endpoint calculation for a service to go to a dual stack (ipv6 enabled) endpoint
/// for the configured region.
Expand Down Expand Up @@ -1080,7 +1039,7 @@ public int EndpointDiscoveryCacheLimit
/// <summary>
/// Returns the flag indicating the current mode in use for request
/// retries and influences the value returned from <see cref="MaxErrorRetry"/>.
/// The default value is RequestRetryMode.Legacy. This flag can be configured
/// The default value is <see cref="RequestRetryMode.Standard"/>. This flag can be configured
/// by using the AWS_RETRY_MODE environment variable, retry_mode in the
/// shared configuration file, or by setting this value directly.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ internal Dictionary<string, Dictionary<string, string>> NestedProperties
/// The desired <see cref="DefaultConfiguration.Name"/> that
/// <see cref="IDefaultConfigurationProvider"/> should use.
/// <para />
/// If this is null/empty, then the <see cref="DefaultConfigurationMode.Legacy"/> Mode will be used.
/// If this is null/empty, then the <see cref="DefaultConfigurationMode.Standard"/> Mode will be used.
/// </summary>
public string DefaultConfigurationModeName { get; set; }

Expand Down Expand Up @@ -105,7 +105,7 @@ internal Dictionary<string, Dictionary<string, string>> NestedProperties
public S3UsEast1RegionalEndpointValue? S3RegionalEndpoint { get; set; }

/// <summary>
/// The request retry mode as legacy, standard, or adaptive
/// The request retry mode as standard or adaptive
/// </summary>
public RequestRetryMode? RetryMode { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ private bool TryGetProfile(string profileName, bool doRefresh, bool isSsoSession
{
if (!Enum.TryParse<RequestRetryMode>(retryModeString, true, out var retryModeTemp))
{
_logger.InfoFormat("Invalid value {0} for {1} in profile {2}. A string legacy/standard/adaptive is expected.", retryModeString, RetryModeField, profileName);
_logger.InfoFormat("Invalid value {0} for {1} in profile {2}. A string standard/adaptive is expected.", retryModeString, RetryModeField, profileName);
profile = null;
return false;
}
Expand Down
10 changes: 3 additions & 7 deletions sdk/src/Core/Amazon.Runtime/DefaultConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,8 @@ namespace Amazon.Runtime
/// <para />
/// All options above can be configured by users, and the overridden value will take precedence.
/// <para />
/// <b>Note:</b> for any mode other than <see cref="DefaultConfigurationMode.Legacy"/>, the vended default values
/// might change as best practices may evolve. As a result, it is encouraged to perform testing when upgrading the SDK
/// if you are using a mode other than <see cref="DefaultConfigurationMode.Legacy"/>.
/// <para />
/// While the <see cref="DefaultConfigurationMode.Legacy"/> defaults mode is specific to .NET,
/// other modes are standardized across all of the AWS SDKs.
/// <b>Note:</b> the vended default values might change as best practices may evolve. As a result, it is encouraged to perform
/// testing when upgrading the SDK.
/// <para />
/// The defaults mode can be configured:
/// <list type="number">
Expand All @@ -41,7 +37,7 @@ namespace Amazon.Runtime
public interface IDefaultConfiguration
{
/// <summary>
/// Identifies a specific configuration mode. Example legacy, mobile, cross-region, etc
/// Identifies a specific configuration mode. Example mobile, cross-region, etc
/// </summary>
DefaultConfigurationMode Name { get; }
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ public enum DefaultConfigurationMode
/// <summary>
/// <p>The AUTO mode is an experimental mode that builds on the standard mode. The SDK will attempt to discover the execution environment to determine the appropriate settings automatically.</p><p>Note that the auto detection is heuristics-based and does not guarantee 100% accuracy. STANDARD mode will be used if the execution environment cannot be determined. The auto detection might query <a href="https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html">EC2 Instance Metadata service</a>, which might introduce latency. Therefore we recommend choosing an explicit defaults_mode instead if startup latency is critical to your application</p>
/// </summary>
Auto,
/// <summary>
/// <p>The LEGACY mode provides default settings that vary per SDK and were used prior to establishment of defaults_mode</p>
/// </summary>
Legacy
Auto
}
}
4 changes: 0 additions & 4 deletions sdk/src/Core/Amazon.Runtime/Enumerations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,6 @@ public enum S3UsEast1RegionalEndpointValue
/// </summary>
public enum RequestRetryMode
{
/// <summary>
/// Legacy request retry strategy.
/// </summary>
Legacy,
/// <summary>
/// Standardized request retry strategy that is consistent across all SDKs.
/// </summary>
Expand Down
5 changes: 2 additions & 3 deletions sdk/src/Core/Amazon.Runtime/IClientConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,9 @@ public partial interface IClientConfig

/// <summary>
/// Returns the flag indicating how many retry HTTP requests an SDK should
/// make for a single SDK operation invocation before giving up. This flag will
/// return 4 when the RetryMode is set to "Legacy" which is the default. For
/// make for a single SDK operation invocation before giving up. For
/// RetryMode values of "Standard" or "Adaptive" this flag will return 2. In
/// addition to the values returned that are dependant on the RetryMode, the
/// addition to the values returned that are dependent on the RetryMode, the
/// value can be set to a specific value by using the AWS_MAX_ATTEMPTS environment
/// variable, max_attempts in the shared configuration file, or by setting a
/// value directly on this property. When using AWS_MAX_ATTEMPTS or max_attempts
Expand Down
Loading