Only get package version once (#928)

* Only get package version once

* Add disconnect method

* Add disconnect method

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Author: hmstepanek

newrelic/hooks/datastore_aioredis.py

@@ -22,6 +22,8 @@
     _redis_operation_re,
 )
 
+AIOREDIS_VERSION = get_package_version_tuple("aioredis")
+
 
 def _conn_attrs_to_dict(connection):
     host = getattr(connection, "host", None)
@@ -58,14 +60,13 @@ def _nr_wrapper_AioRedis_method_(wrapped, instance, args, kwargs):
         # Check for transaction and return early if found.
         # Method will return synchronously without executing,
         # it will be added to the command stack and run later.
-        aioredis_version = get_package_version_tuple("aioredis")
 
         # This conditional is for versions of aioredis that are outside
         # New Relic's supportability window but will still work.  New
         # Relic does not provide testing/support for this.  In order to
         # keep functionality without affecting coverage metrics, this
         # segment is excluded from coverage analysis.
-        if aioredis_version and aioredis_version < (2,):  # pragma: no cover
+        if AIOREDIS_VERSION and AIOREDIS_VERSION < (2,):  # pragma: no cover
             # AioRedis v1 uses a RedisBuffer instead of a real connection for queueing up pipeline commands
             from aioredis.commands.transaction import _RedisBuffer
 
@@ -75,7 +76,7 @@ def _nr_wrapper_AioRedis_method_(wrapped, instance, args, kwargs):
                 return wrapped(*args, **kwargs)
         else:
             # AioRedis v2 uses a Pipeline object for a client and internally queues up pipeline commands
-            if aioredis_version:
+            if AIOREDIS_VERSION:
                 from aioredis.client import Pipeline
             if isinstance(instance, Pipeline):
                 return wrapped(*args, **kwargs)
@@ -139,6 +140,7 @@ async def wrap_Connection_send_command(wrapped, instance, args, kwargs):
     ):
         return await wrapped(*args, **kwargs)
 
+
 # This wrapper is for versions of aioredis that are outside
 # New Relic's supportability window but will still work.  New
 # Relic does not provide testing/support for this.  In order to

tests/datastore_redis/test_custom_conn_pool.py

@@ -12,22 +12,22 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-''' The purpose of these tests is to confirm that using a non-standard
+""" The purpose of these tests is to confirm that using a non-standard
 connection pool that does not have a `connection_kwargs` attribute
 will not result in an error.
-'''
+"""
 
 import pytest
 import redis
-
-from newrelic.api.background_task import background_task
-from newrelic.common.package_version_utils import get_package_version_tuple
-
-from testing_support.fixtures import override_application_settings
-from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics
 from testing_support.db_settings import redis_settings
+from testing_support.fixtures import override_application_settings
 from testing_support.util import instance_hostname
+from testing_support.validators.validate_transaction_metrics import (
+    validate_transaction_metrics,
+)
 
+from newrelic.api.background_task import background_task
+from newrelic.common.package_version_utils import get_package_version_tuple
 
 DB_SETTINGS = redis_settings()[0]
 REDIS_PY_VERSION = get_package_version_tuple("redis")
@@ -45,13 +45,17 @@ def get_connection(self, name, *keys, **options):
     def release(self, connection):
         self.connection.disconnect()
 
+    def disconnect(self):
+        self.connection.disconnect()
+
+
 # Settings
 
 _enable_instance_settings = {
-    'datastore_tracer.instance_reporting.enabled': True,
+    "datastore_tracer.instance_reporting.enabled": True,
 }
 _disable_instance_settings = {
-    'datastore_tracer.instance_reporting.enabled': False,
+    "datastore_tracer.instance_reporting.enabled": False,
 }
 
 # Metrics
@@ -61,98 +65,100 @@ def release(self, connection):
 datastore_all_metric_count = 5 if REDIS_PY_VERSION >= (5, 0) else 3
 
 _base_scoped_metrics = [
-        ('Datastore/operation/Redis/get', 1),
-        ('Datastore/operation/Redis/set', 1),
-        ('Datastore/operation/Redis/client_list', 1),
+    ("Datastore/operation/Redis/get", 1),
+    ("Datastore/operation/Redis/set", 1),
+    ("Datastore/operation/Redis/client_list", 1),
 ]
 # client_setinfo was introduced in v5.0.0 and assigns info displayed in client_list output
 if REDIS_PY_VERSION >= (5, 0):
-    _base_scoped_metrics.append(('Datastore/operation/Redis/client_setinfo', 2),)
+    _base_scoped_metrics.append(
+        ("Datastore/operation/Redis/client_setinfo", 2),
+    )
 
 _base_rollup_metrics = [
-        ('Datastore/all', datastore_all_metric_count),
-        ('Datastore/allOther', datastore_all_metric_count),
-        ('Datastore/Redis/all', datastore_all_metric_count),
-        ('Datastore/Redis/allOther', datastore_all_metric_count),
-        ('Datastore/operation/Redis/get', 1),
-        ('Datastore/operation/Redis/set', 1),
-        ('Datastore/operation/Redis/client_list', 1),
+    ("Datastore/all", datastore_all_metric_count),
+    ("Datastore/allOther", datastore_all_metric_count),
+    ("Datastore/Redis/all", datastore_all_metric_count),
+    ("Datastore/Redis/allOther", datastore_all_metric_count),
+    ("Datastore/operation/Redis/get", 1),
+    ("Datastore/operation/Redis/set", 1),
+    ("Datastore/operation/Redis/client_list", 1),
 ]
 if REDIS_PY_VERSION >= (5, 0):
-    _base_rollup_metrics.append(('Datastore/operation/Redis/client_setinfo', 2),)
+    _base_rollup_metrics.append(
+        ("Datastore/operation/Redis/client_setinfo", 2),
+    )
 
-_host = instance_hostname(DB_SETTINGS['host'])
-_port = DB_SETTINGS['port']
+_host = instance_hostname(DB_SETTINGS["host"])
+_port = DB_SETTINGS["port"]
 
-_instance_metric_name = 'Datastore/instance/Redis/%s/%s' % (_host, _port)
+_instance_metric_name = "Datastore/instance/Redis/%s/%s" % (_host, _port)
 
 instance_metric_count = 5 if REDIS_PY_VERSION >= (5, 0) else 3
 
-_enable_rollup_metrics = _base_rollup_metrics.append(
-        (_instance_metric_name, instance_metric_count)
-)
+_enable_rollup_metrics = _base_rollup_metrics.append((_instance_metric_name, instance_metric_count))
 
-_disable_rollup_metrics = _base_rollup_metrics.append(
-        (_instance_metric_name, None)
-)
+_disable_rollup_metrics = _base_rollup_metrics.append((_instance_metric_name, None))
 
 # Operations
 
+
 def exercise_redis(client):
-    client.set('key', 'value')
-    client.get('key')
-    client.execute_command('CLIENT', 'LIST', parse='LIST')
+    client.set("key", "value")
+    client.get("key")
+    client.execute_command("CLIENT", "LIST", parse="LIST")
+
 
 # Tests
 
-@pytest.mark.skipif(REDIS_PY_VERSION < (2, 7),
-        reason='Client list command introduced in 2.7')
+
+@pytest.mark.skipif(REDIS_PY_VERSION < (2, 7), reason="Client list command introduced in 2.7")
 @override_application_settings(_enable_instance_settings)
 @validate_transaction_metrics(
-        'test_custom_conn_pool:test_fake_conn_pool_enable_instance',
-        scoped_metrics=_base_scoped_metrics,
-        rollup_metrics=_enable_rollup_metrics,
-        background_task=True)
+    "test_custom_conn_pool:test_fake_conn_pool_enable_instance",
+    scoped_metrics=_base_scoped_metrics,
+    rollup_metrics=_enable_rollup_metrics,
+    background_task=True,
+)
 @background_task()
 def test_fake_conn_pool_enable_instance():
-    client = redis.StrictRedis(host=DB_SETTINGS['host'],
-            port=DB_SETTINGS['port'], db=0)
+    client = redis.StrictRedis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0)
 
     # Get a real connection
 
-    conn = client.connection_pool.get_connection('GET')
+    conn = client.connection_pool.get_connection("GET")
 
     # Replace the original connection pool with one that doesn't
     # have the `connection_kwargs` attribute.
 
     fake_pool = FakeConnectionPool(conn)
     client.connection_pool = fake_pool
-    assert not hasattr(client.connection_pool, 'connection_kwargs')
+    assert not hasattr(client.connection_pool, "connection_kwargs")
 
     exercise_redis(client)
 
-@pytest.mark.skipif(REDIS_PY_VERSION < (2, 7),
-        reason='Client list command introduced in 2.7')
+
+@pytest.mark.skipif(REDIS_PY_VERSION < (2, 7), reason="Client list command introduced in 2.7")
 @override_application_settings(_disable_instance_settings)
 @validate_transaction_metrics(
-        'test_custom_conn_pool:test_fake_conn_pool_disable_instance',
-        scoped_metrics=_base_scoped_metrics,
-        rollup_metrics=_disable_rollup_metrics,
-        background_task=True)
+    "test_custom_conn_pool:test_fake_conn_pool_disable_instance",
+    scoped_metrics=_base_scoped_metrics,
+    rollup_metrics=_disable_rollup_metrics,
+    background_task=True,
+)
 @background_task()
 def test_fake_conn_pool_disable_instance():
-    client = redis.StrictRedis(host=DB_SETTINGS['host'],
-            port=DB_SETTINGS['port'], db=0)
+    client = redis.StrictRedis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0)
 
     # Get a real connection
 
-    conn = client.connection_pool.get_connection('GET')
+    conn = client.connection_pool.get_connection("GET")
 
     # Replace the original connection pool with one that doesn't
     # have the `connection_kwargs` attribute.
 
     fake_pool = FakeConnectionPool(conn)
     client.connection_pool = fake_pool
-    assert not hasattr(client.connection_pool, 'connection_kwargs')
+    assert not hasattr(client.connection_pool, "connection_kwargs")
 
     exercise_redis(client)