Merge pull request #643 from jdasilva-invitae/uta-pooling-issue

Improve uta pooling

Makefile

@@ -71,17 +71,20 @@ bdist bdist_egg bdist_wheel build sdist install: %:
 #=> test: execute tests
 #=> test-code: test code (including embedded doctests)
 #=> test-docs: test example code in docs
+#=> stest: test a specific test or set of tests, also useful using ipdb
 #=> test-/tag/ -- run tests marked with /tag/
 # TODO: rationalize tags
 # find tests -name \*.py | xargs perl -ln0e 'while (m/@pytest.mark.(\w+)/g) {print $1 if not $seen{$1}++}'  | sort
 # => extra fx issues mapping models normalization parametrize pnd quick regression validation
-.PHONY: test test-code test-docs
+.PHONY: test test-code test-docs stest
 test:
 	python setup.py pytest
 test-code:
 	python setup.py pytest --addopts="${TEST_DIRS}"
 test-docs:
 	python setup.py pytest --addopts="${DOC_TESTS}"
+stest:
+	python setup.py pytest --addopts="-vvv -s -k ${t}"
 test-%:
 	python setup.py pytest --addopts="-m '$*' ${TEST_DIRS}"
 

hgvs/dataproviders/uta.py

@@ -623,9 +623,13 @@ def _get_cursor(self, n_retries=1):
                 _logger.warning(
                     "Lost connection to {url}; attempting reconnect".format(url=self.url))
                 if self.pooling:
-                    self._pool.closeall()
-                self._connect()
-                _logger.warning("Reconnected to {url}".format(url=self.url))
+                    self._pool.putconn(conn)
+                    _logger.warning(
+                        "Put away pool connection from {url}".format(url=self.url)
+                    )
+                else:
+                    self._connect()
+                    _logger.warning("Reconnected to {url}".format(url=self.url))
 
             n_tries_rem -= 1
 

hgvs/parser.py

@@ -29,7 +29,7 @@
 
 
 class Parser(object):
-    """Provides comprehensive parsing of HGVS varaint strings (*i.e.*,
+    """Provides comprehensive parsing of HGVS variant strings (*i.e.*,
     variants represented according to the Human Genome Variation
     Society recommendations) into Python representations.  The class
     wraps a Parsing Expression Grammar, exposing rules of that grammar

tests/test_hgvs_dataproviders_uta.py

@@ -6,10 +6,13 @@
 import os
 import re
 import unittest
+from unittest.mock import patch
 
-from hgvs.exceptions import HGVSDataNotAvailableError
+import psycopg2
+import pytest
 import hgvs.dataproviders.uta
 import hgvs.edit
+from hgvs.exceptions import HGVSError, HGVSDataNotAvailableError
 import hgvs.location
 import hgvs.posedit
 import hgvs.variantmapper
@@ -104,6 +107,18 @@ def setUpClass(cls):
             pooling=True, mode=os.environ.get("HGVS_CACHE_MODE", "run"), cache=CACHE)
 
 
+class Test_hgvs_dataproviders_uta_with_pooling_without_cache(
+    unittest.TestCase, UTA_Base
+):
+    """
+    Currently used to test pool errors, since we need to reach out to
+    the database and not use the cache
+    """
+    @classmethod
+    def setUpClass(cls):
+        cls.hdp = hgvs.dataproviders.uta.connect(pooling=True)
+
+
 class TestUTACache(Test_hgvs_dataproviders_uta_UTA_default):
     def _create_cdna_variant(self):
         start = hgvs.location.SimplePosition(118898437)
@@ -128,6 +143,24 @@ def test_deterministic_cache_results(self):
         self.assertEqual(str(var1), str(var2))
 
 
+class TestUTAPool(Test_hgvs_dataproviders_uta_with_pooling_without_cache):
+    def test_use_putconn_on_lost_conn(self):
+        """
+        Check that it won't closeall instead of putconn for pooling
+        when it loses the connection
+        """
+        def raise_operational_error(*args, **kwargs):
+            raise psycopg2.OperationalError()
+
+        with patch.object(self.hdp, "_pool") as mock_pool:
+            mock_getconn = mock_pool.getconn.return_value
+            mock_putconn = mock_pool.putconn
+            mock_getconn.cursor.side_effect = raise_operational_error
+            with pytest.raises(HGVSError):
+                self.hdp.get_gene_info("PAH")
+            mock_putconn.assert_called_with(mock_getconn)
+
+
 if __name__ == "__main__":
     unittest.main()