fix review comments

snowflakedb · Aug 26, 2024 · af4daf7 · af4daf7
1 parent 4e2094b
commit af4daf7
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 21 deletions.
diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
@@ -2342,22 +2342,21 @@ public void TestHangingCloseIsNotBlocking()
             var restRequester = new MockCloseHangingRestRequester();
             var session = new SFSession("account=test;user=test;password=test", null, restRequester);
             session.Open();
-            var watch = new Stopwatch();
+            var watchClose = new Stopwatch();
+            var watchClosedFinished = new Stopwatch();
 
             // act
-            watch.Start();
-            session.closeNonBlocking();
-            watch.Stop();
-            var closeDuration = watch.Elapsed.Duration();
-            watch.Restart();
+            watchClose.Start();
+            watchClosedFinished.Start();
+            session.CloseNonBlocking();
+            watchClose.Stop();
             Awaiter.WaitUntilConditionOrTimeout(() => restRequester.CloseRequests.Count > 0, TimeSpan.FromSeconds(15));
-            watch.Stop();
-            var backgroundTaskDuration = watch.Elapsed.Duration();
+            watchClosedFinished.Stop();
 
             // assert
             Assert.AreEqual(1, restRequester.CloseRequests.Count);
-            Assert.Less(closeDuration, TimeSpan.FromSeconds(5)); // close executed immediately
-            Assert.GreaterOrEqual(backgroundTaskDuration, TimeSpan.FromSeconds(10)); // while background task took more time
+            Assert.Less(watchClose.Elapsed.Duration(), TimeSpan.FromSeconds(5)); // close executed immediately
+            Assert.GreaterOrEqual(watchClosedFinished.Elapsed.Duration(), TimeSpan.FromSeconds(10)); // while background task took more time
         }
     }
 }

diff --git a/Snowflake.Data.Tests/UnitTests/SFSessionTest.cs b/Snowflake.Data.Tests/UnitTests/SFSessionTest.cs
@@ -28,7 +28,7 @@ public void TestSessionGoneWhenCloseNonBlocking()
             var restRequester = new MockCloseSessionGone();
             SFSession sfSession = new SFSession("account=test;user=test;password=test", null, restRequester);
             sfSession.Open();
-            Assert.DoesNotThrow(() => sfSession.closeNonBlocking());
+            Assert.DoesNotThrow(() => sfSession.CloseNonBlocking());
         }
 
         [Test]

diff --git a/Snowflake.Data/Client/SnowflakeDbConnection.cs b/Snowflake.Data/Client/SnowflakeDbConnection.cs
@@ -397,7 +397,7 @@ protected override void Dispose(bool disposing)
                 }
                 else
                 {
-                    SfSession?.closeNonBlocking();
+                    SfSession?.CloseNonBlocking();
                     SfSession = null;
                     _connectionState = ConnectionState.Closed;
                 }

diff --git a/Snowflake.Data/Core/Session/SFSession.cs b/Snowflake.Data/Core/Session/SFSession.cs
@@ -281,19 +281,17 @@ internal void close()
             stopHeartBeatForThisSession();
             var closeSessionRequest = PrepareCloseSessionRequest();
             PostCloseSession(closeSessionRequest, restRequester);
-            // Just in case the session won't be closed twice
             sessionToken = null;
         }
 
-        internal void closeNonBlocking()
+        internal void CloseNonBlocking()
         {
             // Nothing to do if the session is not open
             if (!IsEstablished()) return;
             logger.Debug($"Closing session with id: {sessionId}, user: {_user}, database: {database}, schema: {schema}, role: {role}, warehouse: {warehouse}, connection start timestamp: {_startTime}");
             stopHeartBeatForThisSession();
             var closeSessionRequest = PrepareCloseSessionRequest();
             Task.Run(() => PostCloseSession(closeSessionRequest, restRequester));
-            // Just in case the session won't be closed twice
             sessionToken = null;
         }
 
@@ -306,34 +304,33 @@ internal async Task CloseAsync(CancellationToken cancellationToken)
 
             var closeSessionRequest = PrepareCloseSessionRequest();
 
-            logger.Debug($"Send async closeSessionRequest");
+            logger.Debug($"Closing session async");
             var response = await restRequester.PostAsync<CloseResponse>(closeSessionRequest, cancellationToken).ConfigureAwait(false);
             if (!response.success)
             {
-                logger.Error($"Failed to delete session {sessionId}, error ignored. Code: {response.code} Message: {response.message}");
+                logger.Error($"Failed to close session {sessionId}, error ignored. Code: {response.code} Message: {response.message}");
             }
 
             logger.Debug($"Session closed: {sessionId}");
-            // Just in case the session won't be closed twice
             sessionToken = null;
         }
 
         private static void PostCloseSession(SFRestRequest closeSessionRequest, IRestRequester restRequester)
         {
             try
             {
-                logger.Debug($"Send closeSessionRequest");
+                logger.Debug($"Closing session");
                 var response = restRequester.Post<CloseResponse>(closeSessionRequest);
                 if (!response.success)
                 {
-                    logger.Error($"Failed to delete session: {closeSessionRequest.sid}, error ignored. Code: {response.code} Message: {response.message}");
+                    logger.Error($"Failed to close session: {closeSessionRequest.sid}, error ignored. Code: {response.code} Message: {response.message}");
                 }
 
                 logger.Debug($"Session closed: {closeSessionRequest.sid}");
             }
             catch (Exception)
             {
-                logger.Error($"Failed to delete session: {closeSessionRequest.sid}, because of exception.");
+                logger.Error($"Failed to close session: {closeSessionRequest.sid}, because of exception.");
                 throw;
             }
         }