Merge b3f905c into 0e9f919

CHANGELOG.rst

@@ -7,6 +7,25 @@ Change Log
 ----------
 
 
+1.3.1
+=====
+
+**PR 117: Repair handling of sentry_dsn in deployment_utils (C4-361)**
+
+* Fixes to ``deployment_utils``:
+
+  * Changes the handling of sentry DSN as an argument (``--sentry_dsn``)
+    to the deployer.
+  * Doesn't raise an error if environment variables collide but with the same value.
+  * Uses better binding technology for binding environment variables.
+  * Factors in a change to the tests to not use a deprecated
+    name (Deployer changed to IniFileMaker) for one of the classes.
+
+* Fixes to ``qa_utils``:
+
+  * Don't do changelog cross-check for beta versions.
+
+
 1.3.0
 =====
 

dcicutils/deployment_utils.py

@@ -40,6 +40,8 @@ def main():
     FF_ENV_INDEXER, CGAP_ENV_INDEXER, is_indexer_env, indexer_env_for_env,
 )
 from dcicutils.misc_utils import PRINT, Retry, apply_dict_overrides
+from dcicutils.qa_utils import override_environ
+
 
 # constants associated with EB-related APIs
 EB_CONFIGURATION_SETTINGS = 'ConfigurationSettings'
@@ -402,7 +404,7 @@ class IniFileManager:
     def build_ini_file_from_template(cls, template_file_name, init_file_name,
                                      bs_env=None, bs_mirror_env=None, s3_bucket_env=None,
                                      data_set=None, es_server=None, es_namespace=None,
-                                     indexer=None, index_server=None):
+                                     indexer=None, index_server=None, sentry_dsn=None):
         """
         Builds a .ini file from a given template file.
 
@@ -427,7 +429,8 @@ def build_ini_file_from_template(cls, template_file_name, init_file_name,
                                                data_set=data_set,
                                                es_server=es_server,
                                                es_namespace=es_namespace,
-                                               indexer=indexer)
+                                               indexer=indexer,
+                                               sentry_dsn=sentry_dsn)
 
     # Ref: https://stackoverflow.com/questions/19911123/how-can-you-get-the-elastic-beanstalk-application-version-in-your-application  # noqa: E501
     EB_MANIFEST_FILENAME = "/opt/elasticbeanstalk/deploy/manifest"
@@ -549,20 +552,18 @@ def build_ini_stream_from_template(cls, template_file_name, init_file_stream,
         # if we specify an indexer name for bs_env, we did the deployment wrong and should bail
         if bs_env in INDEXER_ENVS:
             raise RuntimeError("Deployed with bs_env %s, which is an indexer env."
-                               "Re-deploy with the env you want to index and set the 'ENCODED_INDEXER'"
-                               "environment variable." % bs_env)
+                               " Re-deploy with the env you want to index and set the 'ENCODED_INDEXER'"
+                               " environment variable." % bs_env)
 
         # We assume these variables are not set, but best to check first. Confusion might result otherwise.
-        for extra_var in extra_vars:
-            if extra_var in os.environ:
-                raise RuntimeError("The environment variable %s is already set to %s."
-                                   % (extra_var, os.environ[extra_var]))
-
-        try:
+        for extra_var, extra_var_val in extra_vars.items():
+            if extra_var in os.environ and extra_var_val != os.environ[extra_var]:
+                raise RuntimeError("The environment variable %s is already set to %s,"
+                                   " but you are trying to set it to %s."
+                                   % (extra_var, os.environ[extra_var], extra_var_val))
 
-            # When we've checked everything, go ahead and do the bindings.
-            for var, val in extra_vars.items():
-                os.environ[var] = val
+        # When we've checked everything, go ahead and do the bindings.
+        with override_environ(**extra_vars):
 
             with io.open(template_file_name, 'r') as template_fp:
                 for line in template_fp:
@@ -575,13 +576,6 @@ def build_ini_stream_from_template(cls, template_file_name, init_file_stream,
                     if not cls.omittable(line, expanded_line):
                         init_file_stream.write(expanded_line)
 
-        finally:
-
-            for key in extra_vars.keys():
-                # Let's be tidy and put things back the way they were before.
-                # Most things probably don't care, but testing might.
-                del os.environ[key]
-
     @classmethod
     def any_environment_template_filename(cls):
         file = os.path.join(cls.TEMPLATE_DIR, "any.ini")
@@ -664,6 +658,9 @@ def main(cls):
                                 help="whether this is a standalone indexing server, only doing indexing",
                                 choices=["true", "false"],
                                 default=None)
+            parser.add_argument("--sentry_dsn",
+                                help="a sentry DSN",
+                                default=None)
             args = parser.parse_args()
             template_file_name = (cls.any_environment_template_filename()
                                   if args.use_any
@@ -675,7 +672,8 @@ def main(cls):
                                              bs_env=args.bs_env, bs_mirror_env=args.bs_mirror_env,
                                              s3_bucket_env=args.s3_bucket_env, data_set=args.data_set,
                                              es_server=args.es_server, es_namespace=args.es_namespace,
-                                             indexer=args.indexer, index_server=args.index_server)
+                                             indexer=args.indexer, index_server=args.index_server,
+                                             sentry_dsn=args.sentry_dsn)
         except Exception as e:
             PRINT("Error (%s): %s" % (e.__class__.__name__, e))
             sys.exit(1)

dcicutils/qa_utils.py

@@ -743,10 +743,17 @@ def _check_version(cls):
         return version
 
     VERSION_LINE_PATTERN = re.compile("^[#* ]*([0-9]+[.][^ \t\n]*)([ \t\n].*)?$")
+    VERSION_IS_BETA_PATTERN = re.compile("^.*[0-9][Bb][0-9]+$")
 
     @classmethod
     def _check_change_history(cls, version=None):
 
+        if version and cls.VERSION_IS_BETA_PATTERN.match(version):
+            # Don't require beta versions to match up in change log.
+            # We don't just strip the version and look at that because sometimes we use other numbers on betas.
+            # Better to just not do it at all.
+            return
+
         changelog_file = getattr_customized(cls, "CHANGELOG")
 
         if not changelog_file:

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "dcicutils"
-version = "1.3.0"
+version = "1.3.0.1b0" # to become "1.3.1"
 description = "Utility package for interacting with the 4DN Data Portal and other 4DN resources"
 authors = ["4DN-DCIC Team <support@4dnucleome.org>"]
 license = "MIT"

test/test_deployment_utils.py

@@ -9,7 +9,7 @@
 from io import StringIO
 from unittest import mock
 
-from dcicutils.deployment_utils import EBDeployer, Deployer, boolean_setting, CreateMappingOnDeployManager
+from dcicutils.deployment_utils import EBDeployer, IniFileManager, boolean_setting, CreateMappingOnDeployManager
 from dcicutils.env_utils import is_cgap_env
 from dcicutils.misc_utils import ignored
 from dcicutils.qa_utils import override_environ
@@ -22,7 +22,7 @@ class FakeDistribution:
     version = "simulated"
 
 
-class TestDeployer(Deployer):
+class TestDeployer(IniFileManager):
     TEMPLATE_DIR = os.path.join(_MY_DIR, "ini_files")
     PYPROJECT_FILE_NAME = os.path.join(os.path.dirname(_MY_DIR), "pyproject.toml")
 
@@ -794,7 +794,7 @@ def mocked_fail(*arg, **kwargs):
                 raise AssertionError("ENV_NAME=None did not get noticed.")
             mock_argparser.side_effect = mocked_fail
             with pytest.raises(SystemExit):
-                Deployer.main()
+                IniFileManager.main()
             assert mock_argparser.call_count == 0
 
 
@@ -805,11 +805,11 @@ def test_deployment_utils_main():
 
     fake_template = "something.ini"  # It doesn't matter what we use as a template for this test. we don't open it.
     with override_environ(ENV_NAME='fourfront-foo'):
-        with mock.patch.object(Deployer, "build_ini_file_from_template") as mock_build:
+        with mock.patch.object(IniFileManager, "build_ini_file_from_template") as mock_build:
             # These next two mocks are just incidental to offering help in arg parsing.
             # Those functions are tested elsewhere and are just plain bypassed here.
-            with mock.patch.object(Deployer, "environment_template_filename", return_value=fake_template):
-                with mock.patch.object(Deployer, "template_environment_names", return_value=["something, foo"]):
+            with mock.patch.object(IniFileManager, "environment_template_filename", return_value=fake_template):
+                with mock.patch.object(IniFileManager, "template_environment_names", return_value=["something, foo"]):
 
                     # This function is the core fo the testing, which just sets up a deployer to get called
                     # with an input template name and a target filename, and then calls the Deployer.
@@ -819,7 +819,7 @@ def mocked_build(*args, **kwargs):
                             assert kwargs == (expected_kwargs or {})
                         mock_build.side_effect = mocked_build
                         try:
-                            Deployer.main()
+                            IniFileManager.main()
                         except SystemExit as e:
                             assert e.code == expected_code
 
@@ -837,7 +837,8 @@ def mocked_build(*args, **kwargs):
                             'es_server': None,
                             'index_server': None,
                             'indexer': None,
-                            's3_bucket_env': None
+                            's3_bucket_env': None,
+                            'sentry_dsn': None,
                         })
 
                     # Next 2 tests some sample settings, in particular the settings of indexer and index_server
@@ -852,7 +853,8 @@ def mocked_build(*args, **kwargs):
                             'es_server': None,
                             'index_server': 'true',
                             'indexer': 'false',
-                            's3_bucket_env': None
+                            's3_bucket_env': None,
+                            'sentry_dsn': None,
                         })
 
                     with mock.patch.object(sys, "argv", ['', '--indexer', 'foo']):
@@ -865,7 +867,8 @@ def mocked_build(*args, **kwargs):
                                 'es_server': None,
                                 'index_server': 'true',
                                 'indexer': 'false',
-                                's3_bucket_env': None
+                                's3_bucket_env': None,
+                                'sentry_dsn': None,
                             })
 
 
@@ -883,7 +886,7 @@ def test_deployment_utils_boolean_setting():
 @pytest.mark.integrated
 def test_eb_deployer():
     """ Tests some basic aspects of EBDeployer """
-    pass  # write this test!
+    pass  # TODO: write this test!
 
 
 class MockedNoCommandArgs:
@@ -1030,7 +1033,6 @@ def test_get_deployment_config_ff_hotseat_old():
                                ' Processing mode: SKIP')
 
 
-
 @mock.patch('dcicutils.deployment_utils.compute_ff_prd_env', mock.MagicMock(return_value='fourfront-green'))
 def test_get_deployment_config_ff_hotseat_new():
     """ Tests get_deployment_config in the hotseat case with a new-style ecosystem. """
@@ -1045,7 +1047,6 @@ def test_get_deployment_config_ff_hotseat_new():
                                ' Processing mode: SKIP')
 
 
-
 # There is no old-style cgap staging
 
 # Eventually cgap staging will look like this.
@@ -1063,7 +1064,6 @@ def test_get_deployment_config_cgap_staging_new():
                                ' Processing mode: STRICT,WIPE_ES')
 
 
-
 @mock.patch('dcicutils.deployment_utils.compute_cgap_prd_env', mock.MagicMock(return_value='fourfront-cgap'))
 def test_get_deployment_config_cgap_prod_old():
     """ Tests get_deployment_config in the old production case """

test/test_qa_utils.py

@@ -1198,16 +1198,9 @@ class MyVersionChecker(VersionChecker):
 
 def test_version_checker_with_missing_changelog():
 
-    path_exists = os.path.exists
+    mfs = MockFileSystem(files={'pyproject.toml': '[tool.poetry]\nname = "foo"\nversion = "1.2.3"'})
 
-    def mocked_exists(filename):
-        if filename.endswith("CHANGELOG.rst"):
-            print("Faking that %s does not exist." % filename)
-            return False
-        else:
-            return path_exists(filename)
-
-    with mock.patch("os.path.exists", mocked_exists):
+    with mock.patch("os.path.exists", mfs.exists):
 
         class MyVersionChecker(VersionChecker):