fix(csharp/src): handle HTTP authorization exception for Thrift-based drivers #3551
Conversation
birschick-bq
changed the title
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks, this is great -- and the bug in AdbcException is a bit embarrassing. I left two questions.
| </ItemGroup> | ||
| <ItemGroup Condition="!$([MSBuild]::IsTargetFrameworkCompatible($(TargetFramework), 'net6.0'))"> | ||
| <PackageReference Include="System.Text.Json" Version="8.0.5" /> | ||
| <PackageReference Include="System.Text.Json" Version="9.0.0" /> |
There was a problem hiding this comment.
Is there a specific need to update System.Text.Json and ApacheThrift (or benefit from updating them)? And why System.Text.Json 9.0.0 vs 9.0.9?
If there's not a specific need to update package versions then I'd just as soon not do it as it can be mildly disruptive downstream.
There was a problem hiding this comment.
I updated the Apache.Thrift to version 0.22.0 because I needed the changes I made to set the InnerException on TTransportExeptions. Otherwise, I would just have to look at the text of the exception message.
Well to use Thrift 0.22.0, it updates the Microsoft.Extension.Loggin.Console to >= 9.0.0 which in turn requires System.Text.JSon >= 9.0.0
I could not find a way to keep System.Text.Json to 8.0.5
There was a problem hiding this comment.
Okay, if we need the new version then we need the new version. We should use the same version of System.Text.Json throughout.
There was a problem hiding this comment.
Okay. Set the System.Text.Json to version 9.0.9 in all referencing projects.
2 participants
C# Thrift-based drivers using HTTP transport don't throw AdbcException with AdbcStatusCode of Unauthorized.
As a best practice, drivers should indicate if the connection encountered an authentication or authorization error when attempting to open. Connection should throw an AdbcException with Status == AdbcStatusCode.Unauthorized.
This handles all Apache (Hive, Impala, Spark) and Databricks drivers that use HTTP transport.
Notes:
closes #3550