Skip to content

Commit e223239

Browse files
committed
Update DatabricksConnection.cs
address comments
1 parent a7d5bcc commit e223239

1 file changed

Lines changed: 16 additions & 14 deletions

File tree

csharp/src/Drivers/Databricks/DatabricksConnection.cs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,22 @@ namespace Apache.Arrow.Adbc.Drivers.Databricks
3030
{
3131
internal class DatabricksConnection : SparkHttpConnection
3232
{
33+
private const bool DefaultRetryOnUnavailable= true;
34+
private const int DefaultTemporarilyUnavailableRetryTimeout = 500;
35+
3336
public DatabricksConnection(IReadOnlyDictionary<string, string> properties) : base(properties)
3437
{
3538
}
3639

3740
/// <summary>
3841
/// Gets a value indicating whether to retry requests that receive a 503 response with a Retry-After header.
3942
/// </summary>
40-
protected bool TemporarilyUnavailableRetry { get; private set; } = true;
43+
protected bool TemporarilyUnavailableRetry { get; private set; } = DefaultRetryOnUnavailable;
4144

4245
/// <summary>
4346
/// Gets the maximum total time in seconds to retry 503 responses before failing.
4447
/// </summary>
45-
protected int TemporarilyUnavailableRetryTimeout { get; private set; } = 900;
48+
protected int TemporarilyUnavailableRetryTimeout { get; private set; } = DefaultTemporarilyUnavailableRetryTimeout;
4649

4750
protected override HttpMessageHandler CreateHttpHandler()
4851
{
@@ -114,14 +117,17 @@ protected override void ValidateOptions()
114117
{
115118
base.ValidateOptions();
116119

117-
bool tempUnavailableRetryValue = true; // Default to enabled
118-
// Parse retry configuration parameters
119-
if(Properties.TryGetValue(DatabricksParameters.TemporarilyUnavailableRetry, out string? tempUnavailableRetryStr))
120-
{
121-
throw new ArgumentOutOfRangeException(DatabricksParameters.TemporarilyUnavailableRetry, tempUnavailableRetryStr,
122-
$"must be a value of false (disabled) or true (enabled). Default is 1.");
123-
}
124-
TemporarilyUnavailableRetry = tempUnavailableRetryValue;
120+
if (Properties.TryGetValue(DatabricksParameters.TemporarilyUnavailableRetry, out string? tempUnavailableRetryStr))
121+
{
122+
if (!bool.TryParse(tempUnavailableRetryStr, out bool tempUnavailableRetryValue))
123+
{
124+
throw new ArgumentOutOfRangeException(DatabricksParameters.TemporarilyUnavailableRetry, tempUnavailableRetryStr,
125+
$"must be a value of false (disabled) or true (enabled). Default is true.");
126+
}
127+
128+
TemporarilyUnavailableRetry = tempUnavailableRetryValue;
129+
}
130+
125131

126132
if(Properties.TryGetValue(DatabricksParameters.TemporarilyUnavailableRetryTimeout, out string? tempUnavailableRetryTimeoutStr))
127133
{
@@ -133,10 +139,6 @@ protected override void ValidateOptions()
133139
}
134140
TemporarilyUnavailableRetryTimeout = tempUnavailableRetryTimeoutValue;
135141
}
136-
else
137-
{
138-
TemporarilyUnavailableRetryTimeout = 900; // Default to 15 minutes
139-
}
140142
}
141143

142144
protected override Task<TGetResultSetMetadataResp> GetResultSetMetadataAsync(TGetSchemasResp response, CancellationToken cancellationToken = default) =>

0 commit comments

Comments
 (0)