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.

Sign up for GitHub

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

SNOW-950923 queryid for put/get statement #828

Closed
wants to merge 6 commits into from
Closed

SNOW-950923 queryid for put/get statement #828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4d2421f
SNOW-950923 fix to provide QueryID for failures during GET/PUT file o…
sfc-gh-mhofman Nov 30, 2023
6ca9c91
SNOW-950923 fix for two different SnowflakeDbExceptions not to collide
sfc-gh-mhofman Dec 5, 2023
b9661c9
SNOW-950923 ensuring any exception throw during file transfer provide…
sfc-gh-mhofman Dec 6, 2023
4aae56e
SNOW-950923 changed type of exception thrown when directory/file not …
sfc-gh-mhofman Dec 6, 2023
13d1106
SNOW-950923 QueryID for PUT/GET without changes to the related except…
sfc-gh-mhofman Dec 6, 2023
c1f9af0
SNOW-950923 Fix for handling errors in tests
sfc-gh-mhofman Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 59 additions & 3 deletions Snowflake.Data.Tests/IntegrationTests/SFPutGetTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,49 @@ public void TestPutFileRelativePathWithoutDirectory()
VerifyFilesAreUploaded(conn, new List<string> { t_inputFilePath }, t_internalStagePath);
}
}

[Test]
public void TestPutFileProvidesQueryIdOnFailure()
{
// Arrange
// Set the PUT query variables but do not create a file
t_inputFilePath = "unexisting_file.csv";
t_internalStagePath = $"@{t_schemaName}.{t_stageName}";

// Act
using (var conn = new SnowflakeDbConnection(ConnectionString))
{
conn.Open();
var queryId = PutFile(conn);

// Assert
Assert.IsNotNull(queryId);
Assert.DoesNotThrow(()=>Guid.Parse(queryId));
}
}

[Test]
public void TestPutFileProvidesQueryIdOnSuccess()
{
// Arrange
// Set the PUT query variables
t_inputFilePath = $"{Guid.NewGuid()}_1.csv";
t_internalStagePath = $"@{t_schemaName}.{t_stageName}";
PrepareFileData(t_inputFilePath);

// Act
using (var conn = new SnowflakeDbConnection(ConnectionString))
{
conn.Open();
var queryId = PutFile(conn);

// Assert
Assert.IsNotNull(queryId);
Assert.DoesNotThrow(()=>Guid.Parse(queryId));
VerifyFilesAreUploaded(conn, new List<string> { t_inputFilePath }, t_internalStagePath);
}
}

[Test]
public void TestPutFileRelativePathWithDirectory()
{
Expand Down Expand Up @@ -459,11 +501,12 @@ private static bool IsCompressedByTheDriver()
}

// PUT - upload file from local directory to the stage
void PutFile(
string PutFile(
SnowflakeDbConnection conn,
String additionalAttribute = "",
ResultStatus expectedStatus = ResultStatus.UPLOADED)
{
String queryId;
using (var command = conn.CreateCommand())
{
// Prepare PUT query
Expand All @@ -474,9 +517,21 @@ void PutFile(

// Upload file
command.CommandText = putQuery;
var reader = command.ExecuteReader();
DbDataReader reader;
try
{
reader = command.ExecuteReader();
}
catch (Exception)
{
queryId = ((SnowflakeDbCommand)command).GetQueryId();
return queryId;
}
// Checking query id when reader succeeded
Assert.IsTrue(reader.Read());

queryId = ((SnowflakeDbDataReader)reader).GetQueryId();
// Checking if query Id is provided on the command level as well
Assert.AreEqual(queryId, ((SnowflakeDbCommand)command).GetQueryId());
// Check file status
Assert.AreEqual(expectedStatus.ToString(),
reader.GetString((int)SFResultSet.PutGetResponseRowTypeInfo.ResultStatus));
Expand All @@ -497,6 +552,7 @@ void PutFile(
}
Assert.IsNull(reader.GetString((int)SFResultSet.PutGetResponseRowTypeInfo.ErrorDetails));
}
return queryId;
}

// COPY INTO - Copy data from the stage into temp table
Expand Down
46 changes: 23 additions & 23 deletions Snowflake.Data/Client/SnowflakeDbException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
static private ResourceManager rm = new ResourceManager("Snowflake.Data.Core.ErrorMessages",
typeof(SnowflakeDbException).Assembly);

public string SqlState { get; private set; }

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, GCP) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, AZURE) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on MAC (net6.0, GCP) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on MAC (net6.0, AZURE) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on MAC (net6.0, AWS) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, AWS) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net6.0, AWS) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net6.0, GCP) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

Check warning on line 26 in Snowflake.Data/Client/SnowflakeDbException.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net6.0, AZURE) 

'SnowflakeDbException.SqlState' hides inherited member 'DbException.SqlState'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.

private int VendorCode;

Expand All @@ -40,55 +40,55 @@
public SnowflakeDbException(string sqlState, int vendorCode, string errorMessage, string queryId)
: base(FormatExceptionMessage(errorMessage, vendorCode, sqlState, queryId))
{
this.SqlState = sqlState;
this.VendorCode = vendorCode;
this.QueryId = queryId;
SqlState = sqlState;
VendorCode = vendorCode;
QueryId = queryId;
}

public SnowflakeDbException(SFError error, params object[] args)
: base(FormatExceptionMessage(error, args, string.Empty, string.Empty))
{
this.VendorCode = error.GetAttribute<SFErrorAttr>().errorCode;
VendorCode = error.GetAttribute<SFErrorAttr>().errorCode;
}

public SnowflakeDbException(string sqlState, SFError error, params object[] args)
: base(FormatExceptionMessage(error, args, sqlState, string.Empty))
{
this.VendorCode = error.GetAttribute<SFErrorAttr>().errorCode;
this.SqlState = sqlState;
VendorCode = error.GetAttribute<SFErrorAttr>().errorCode;
SqlState = sqlState;
}

public SnowflakeDbException(Exception innerException, SFError error, params object[] args)
: base(FormatExceptionMessage(error, args, string.Empty, string.Empty), innerException)
{
this.VendorCode = error.GetAttribute<SFErrorAttr>().errorCode;
VendorCode = error.GetAttribute<SFErrorAttr>().errorCode;
}

public SnowflakeDbException(Exception innerException, string sqlState, SFError error, params object[] args)
: base(FormatExceptionMessage(error, args, sqlState, string.Empty), innerException)
{
this.VendorCode = error.GetAttribute<SFErrorAttr>().errorCode;
this.SqlState = sqlState;
VendorCode = error.GetAttribute<SFErrorAttr>().errorCode;
SqlState = sqlState;
}

static string FormatExceptionMessage(SFError error,
object[] args,
string sqlState,
string queryId)
{
return FormatExceptionMessage(string.Format(rm.GetString(error.ToString()), args)
, error.GetAttribute<SFErrorAttr>().errorCode
, sqlState
, queryId);
}

static string FormatExceptionMessage(string errorMessage,
int vendorCode,
string sqlState,
string queryId)
{
return string.Format("Error: {0} SqlState: {1}, VendorCode: {2}, QueryId: {3}",
errorMessage, sqlState, vendorCode, queryId);
{
return FormatExceptionMessage(string.Format(rm.GetString(error.ToString()), args)
, error.GetAttribute<SFErrorAttr>().errorCode
, sqlState
, queryId);
}

static string FormatExceptionMessage(string errorMessage,
int vendorCode,
string sqlState,
string queryId)
{
return string.Format("Error: {0} SqlState: {1}, VendorCode: {2}, QueryId: {3}",
errorMessage, sqlState, vendorCode, queryId);
}
}
}
2 changes: 1 addition & 1 deletion Snowflake.Data/Core/ErrorMessages.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,4 @@
<data name="BROWSER_RESPONSE_TIMEOUT" xml:space="preserve">
<value>Browser response timed out after {0} seconds.</value>
</data>
</root>
</root>
67 changes: 38 additions & 29 deletions Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,46 +185,55 @@
/// </summary>
public void execute()
{
// Initialize the encryption metadata
initEncryptionMaterial();

if (CommandTypes.UPLOAD == CommandType)
{
initFileMetadataForUpload();
}
else if (CommandTypes.DOWNLOAD == CommandType)
try
{
initFileMetadata(TransferMetadata.src_locations);
// Initialize the encryption metadata
initEncryptionMaterial();

Directory.CreateDirectory(TransferMetadata.localLocation);
}
if (CommandTypes.UPLOAD == CommandType)
{
initFileMetadataForUpload();
}
else if (CommandTypes.DOWNLOAD == CommandType)
{
initFileMetadata(TransferMetadata.src_locations);

// Update the file metadata with GCS presigned URL
updatePresignedUrl();
Directory.CreateDirectory(TransferMetadata.localLocation);
}

foreach (SFFileMetadata fileMetadata in FilesMetas)
{
// If the file is larger than the threshold, add it to the large files list
// Otherwise add it to the small files list
if (fileMetadata.srcFileSize > TransferMetadata.threshold)
// Update the file metadata with GCS presigned URL
updatePresignedUrl();

foreach (SFFileMetadata fileMetadata in FilesMetas)
{
LargeFilesMetas.Add(fileMetadata);
// If the file is larger than the threshold, add it to the large files list
// Otherwise add it to the small files list
if (fileMetadata.srcFileSize > TransferMetadata.threshold)
{
LargeFilesMetas.Add(fileMetadata);
}

Check warning on line 214 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs#L212-L214 

Added lines #L212 - L214 were not covered by tests
else
{
SmallFilesMetas.Add(fileMetadata);
}
}
else

// Check command type
if (CommandTypes.UPLOAD == CommandType)
{
SmallFilesMetas.Add(fileMetadata);
upload();
}
else if (CommandTypes.DOWNLOAD == CommandType)
{
download();
}
}

// Check command type
if (CommandTypes.UPLOAD == CommandType)
{
upload();
}
else if (CommandTypes.DOWNLOAD == CommandType)
catch (Exception e)
{
download();
Logger.Error("Error while transferring file(s): " + e.Message);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it made sense to wrap any exception here like FileNotFound or DirectoryNotFoundException into SnowflakeDbException as in
https://github.com/snowflakedb/snowflake-connector-net/pull/825/files#diff-db3c83b5b538ba4849e37bb0565ec20d836891f35dd60156da30501ea34ec6bfR231-R245
to be able to provide QueryId within exception. However,
it would introduce possibly some changes to the application which might already be catching only those types of exceptions and not SnowflakeDbException or exception in general.
So a change in SFStatement.cs is enough to provide query id in case file transfer error throws anything here.

throw;
}

}

public async Task executeAsync(CancellationToken cancellationToken)
Expand Down Expand Up @@ -693,7 +702,7 @@
{
return int.Parse(maxBytesInMemoryString);
}
catch (Exception e)

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, GCP) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, AZURE) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on MAC (net6.0, GCP) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on MAC (net6.0, AZURE) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on MAC (net6.0, AWS) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, AWS) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net6.0, AWS) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net6.0, GCP) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net6.0, AZURE) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE) 

The variable 'e' is declared but never used

Check warning on line 705 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS) 

The variable 'e' is declared but never used
{
return FileTransferConfiguration.DefaultMaxBytesInMemory;
}
Expand Down Expand Up @@ -892,7 +901,7 @@
/// <param name="fileMetadata">The metadata for the file to get digest.</param>
private void getDigestAndSizeForFile(SFFileMetadata fileMetadata)
{
using (SHA256 SHA256 = SHA256Managed.Create())

Check warning on line 904 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, GCP) 

'SHA256Managed' is obsolete: 'Derived cryptographic types are obsolete. Use the Create method on the base type instead.' (https://aka.ms/dotnet-warnings/SYSLIB0021)

Check warning on line 904 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, AZURE) 

'SHA256Managed' is obsolete: 'Derived cryptographic types are obsolete. Use the Create method on the base type instead.' (https://aka.ms/dotnet-warnings/SYSLIB0021)

Check warning on line 904 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on MAC (net6.0, GCP) 

'SHA256Managed' is obsolete: 'Derived cryptographic types are obsolete. Use the Create method on the base type instead.' (https://aka.ms/dotnet-warnings/SYSLIB0021)

Check warning on line 904 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on MAC (net6.0, AZURE) 

'SHA256Managed' is obsolete: 'Derived cryptographic types are obsolete. Use the Create method on the base type instead.' (https://aka.ms/dotnet-warnings/SYSLIB0021)

Check warning on line 904 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Linux (net6.0, AWS) 

'SHA256Managed' is obsolete: 'Derived cryptographic types are obsolete. Use the Create method on the base type instead.' (https://aka.ms/dotnet-warnings/SYSLIB0021)

Check warning on line 904 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net6.0, AWS) 

'SHA256Managed' is obsolete: 'Derived cryptographic types are obsolete. Use the Create method on the base type instead.' (https://aka.ms/dotnet-warnings/SYSLIB0021)

Check warning on line 904 in Snowflake.Data/Core/FileTransfer/SFFileTransferAgent.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net6.0, AZURE) 

'SHA256Managed' is obsolete: 'Derived cryptographic types are obsolete. Use the Create method on the base type instead.' (https://aka.ms/dotnet-warnings/SYSLIB0021)
{
if (fileMetadata.memoryStream != null)
{
Expand Down
5 changes: 5 additions & 0 deletions Snowflake.Data/Core/SFStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,11 @@ internal SFBaseResultSet Execute(int timeout, string sql, Dictionary<string, Bin
bindings,
describeOnly);

logger.Debug("PUT/GET queryId: " + (response.data != null ? response.data.queryId : "Unknown"));

if (response.data != null)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since query id is provided by the above ExecuteHelper upfront (before we start get/put file operation) we reuse it even before fileTransferAgent throws any possible error.
It's a simpler approach then in #825 @sfc-gh-knozderko
cc @sfc-gh-dszmolka

_lastQueryId = response.data.queryId;

SFFileTransferAgent fileTransferAgent =
new SFFileTransferAgent(trimmedSql, SfSession, response.data, CancellationToken.None);

Expand Down
Loading