reverted SNOW-264606 client session keep alive fix (#664)

Author: sfc-gh-hchaturvedi

src/snowflake/connector/connection.py

@@ -94,7 +94,11 @@
 from .sqlstate import SQLSTATE_CONNECTION_NOT_EXISTS, SQLSTATE_FEATURE_NOT_SUPPORTED
 from .telemetry import TelemetryClient
 from .telemetry_oob import TelemetryService
-from .time_util import HeartBeatTimer, get_time_millis
+from .time_util import (
+    DEFAULT_MASTER_VALIDITY_IN_SECONDS,
+    HeartBeatTimer,
+    get_time_millis,
+)
 from .util_text import construct_hostname, parse_account, split_statements
 
 
@@ -154,7 +158,7 @@ def DefaultConverterClass():
     "inject_client_pause": (0, int),  # snowflake internal
     "session_parameters": (None, (type(None), dict)),  # snowflake session parameters
     "autocommit": (None, (type(None), bool)),  # snowflake
-    "client_session_keep_alive": (None, (type(None), bool)),  # snowflake
+    "client_session_keep_alive": (False, bool),  # snowflake
     "client_session_keep_alive_heartbeat_frequency": (
         None,
         (type(None), int),
@@ -359,11 +363,15 @@ def client_session_keep_alive(self):
 
     @client_session_keep_alive.setter
     def client_session_keep_alive(self, value):
-        self._client_session_keep_alive = value
+        self._client_session_keep_alive = True if value else False
 
     @property
     def client_session_keep_alive_heartbeat_frequency(self):
-        return self._client_session_keep_alive_heartbeat_frequency
+        return (
+            self._client_session_keep_alive_heartbeat_frequency
+            if self._client_session_keep_alive_heartbeat_frequency
+            else DEFAULT_MASTER_VALIDITY_IN_SECONDS / 16
+        )
 
     @client_session_keep_alive_heartbeat_frequency.setter
     def client_session_keep_alive_heartbeat_frequency(self, value):
@@ -665,12 +673,10 @@ def __open_connection(self):
                 PARAMETER_CLIENT_VALIDATE_DEFAULT_PARAMETERS
             ] = True
 
-        if self.client_session_keep_alive is not None:
-            self._session_parameters[
-                PARAMETER_CLIENT_SESSION_KEEP_ALIVE
-            ] = self._client_session_keep_alive
+        if self.client_session_keep_alive:
+            self._session_parameters[PARAMETER_CLIENT_SESSION_KEEP_ALIVE] = True
 
-        if self.client_session_keep_alive_heartbeat_frequency is not None:
+        if self.client_session_keep_alive_heartbeat_frequency:
             self._session_parameters[
                 PARAMETER_CLIENT_SESSION_KEEP_ALIVE_HEARTBEAT_FREQUENCY
             ] = self._validate_client_session_keep_alive_heartbeat_frequency()
@@ -698,9 +704,6 @@ def __open_connection(self):
         self._password = None  # ensure password won't persist
 
         if self.client_session_keep_alive:
-            # This will be called after the heartbeat frequency has actually been set.
-            # By this point it should have been decided if the heartbeat has to be enabled
-            # and what would the heartbeat frequency be
             self._add_heartbeat()
 
     def __preprocess_auth_instance(self, auth_instance):
@@ -1176,10 +1179,7 @@ def _validate_client_session_keep_alive_heartbeat_frequency(self):
         """Validate and return heartbeat frequency in seconds."""
         real_max = int(self.rest.master_validity_in_seconds / 4)
         real_min = int(real_max / 4)
-        if self.client_session_keep_alive_heartbeat_frequency is None:
-            # This is an unlikely scenario but covering it just in case.
-            self._client_session_keep_alive_heartbeat_frequency = real_min
-        elif self.client_session_keep_alive_heartbeat_frequency > real_max:
+        if self.client_session_keep_alive_heartbeat_frequency > real_max:
             self._client_session_keep_alive_heartbeat_frequency = real_max
         elif self.client_session_keep_alive_heartbeat_frequency < real_min:
             self._client_session_keep_alive_heartbeat_frequency = real_min
@@ -1215,15 +1215,8 @@ def _update_parameters(
                 else:
                     TelemetryService.get_instance().disable()
             elif PARAMETER_CLIENT_SESSION_KEEP_ALIVE == name:
-                # Only set if the local config is None.
-                # Always give preference to user config.
-                if self.client_session_keep_alive is None:
-                    self.client_session_keep_alive = value
-            elif (
-                PARAMETER_CLIENT_SESSION_KEEP_ALIVE_HEARTBEAT_FREQUENCY == name
-                and self.client_session_keep_alive_heartbeat_frequency is None
-            ):
-                # Only set if local value hasn't been set already.
+                self.client_session_keep_alive = value
+            elif PARAMETER_CLIENT_SESSION_KEEP_ALIVE_HEARTBEAT_FREQUENCY == name:
                 self.client_session_keep_alive_heartbeat_frequency = value
             elif PARAMETER_SERVICE_NAME == name:
                 self.service_name = value

test/integ/test_session_parameters.py

@@ -4,15 +4,8 @@
 # Copyright (c) 2012-2021 Snowflake Computing Inc. All right reserved.
 #
 
-import pytest
-
 import snowflake.connector
 
-try:  # pragma: no cover
-    from parameters import CONNECTION_PARAMETERS_ADMIN
-except ImportError:
-    CONNECTION_PARAMETERS_ADMIN = {}
-
 
 def test_session_parameters(db_parameters):
     """Sets the session parameters in connection time."""
@@ -29,88 +22,3 @@ def test_session_parameters(db_parameters):
     )
     ret = connection.cursor().execute("show parameters like 'TIMEZONE'").fetchone()
     assert ret[1] == "UTC"
-
-
-@pytest.mark.skipif(
-    not CONNECTION_PARAMETERS_ADMIN,
-    reason="Snowflake admin required to setup parameter.",
-)
-def test_client_session_keep_alive(db_parameters, conn_cnx):
-    """Tests client_session_keep_alive setting.
-
-    Ensures that client's explicit config for client_session_keep_alive
-    session parameter is always honored and given higher precedence over
-    user and account level backend configuration.
-    """
-    admin_cnxn = snowflake.connector.connect(
-        protocol=db_parameters["sf_protocol"],
-        account=db_parameters["sf_account"],
-        user=db_parameters["sf_user"],
-        password=db_parameters["sf_password"],
-        host=db_parameters["sf_host"],
-        port=db_parameters["sf_port"],
-    )
-
-    # Ensure backend parameter is set to False
-    set_backend_client_session_keep_alive(db_parameters, admin_cnxn, False)
-    with conn_cnx(client_session_keep_alive=True) as connection:
-        ret = (
-            connection.cursor()
-            .execute("show parameters like 'CLIENT_SESSION_KEEP_ALIVE'")
-            .fetchone()
-        )
-        assert ret[1] == "true"
-
-    # Set backend parameter to True
-    set_backend_client_session_keep_alive(db_parameters, admin_cnxn, True)
-
-    # Set session parameter to False
-    with conn_cnx(client_session_keep_alive=False) as connection:
-        ret = (
-            connection.cursor()
-            .execute("show parameters like 'CLIENT_SESSION_KEEP_ALIVE'")
-            .fetchone()
-        )
-        assert ret[1] == "false"
-
-    # Set session parameter to None backend parameter continues to be True
-    with conn_cnx(client_session_keep_alive=None) as connection:
-        ret = (
-            connection.cursor()
-            .execute("show parameters like 'CLIENT_SESSION_KEEP_ALIVE'")
-            .fetchone()
-        )
-        assert ret[1] == "true"
-
-    admin_cnxn.close()
-
-
-def create_client_connection(db_parameters: object, val: bool) -> object:
-    """Create connection with client session keep alive set to specific value."""
-    connection = snowflake.connector.connect(
-        protocol=db_parameters["protocol"],
-        account=db_parameters["account"],
-        user=db_parameters["user"],
-        password=db_parameters["password"],
-        host=db_parameters["host"],
-        port=db_parameters["port"],
-        database=db_parameters["database"],
-        schema=db_parameters["schema"],
-        client_session_keep_alive=val,
-    )
-    return connection
-
-
-def set_backend_client_session_keep_alive(
-    db_parameters: object, admin_cnx: object, val: bool
-) -> None:
-    """Set both at Account level and User level."""
-    query = "alter account {} set CLIENT_SESSION_KEEP_ALIVE={}".format(
-        db_parameters["account"], str(val)
-    )
-    admin_cnx.cursor().execute(query)
-
-    query = "alter user {}.{} set CLIENT_SESSION_KEEP_ALIVE={}".format(
-        db_parameters["account"], db_parameters["user"], str(val)
-    )
-    admin_cnx.cursor().execute(query)