From d6a675137baf671e31f80a4615d9c83cc0bf67c7 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 9 May 2023 09:47:55 -0600 Subject: [PATCH 01/10] per #2168, add check to ensure valid time is a datetime object before trying to use it to compute time offset --- metplus/util/time_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metplus/util/time_util.py b/metplus/util/time_util.py index cd53af2511..fed14ec35d 100755 --- a/metplus/util/time_util.py +++ b/metplus/util/time_util.py @@ -159,7 +159,7 @@ def ti_get_seconds_from_relativedelta(lead, valid_time=None): return None # if valid time is specified, use it to determine seconds - if valid_time: + if valid_time and isinstance(valid_time, datetime.datetime): return int((valid_time - (valid_time - lead)).total_seconds()) if lead.months != 0 or lead.years != 0: From f48296df0d1e458d9df6e7330194240a9495d726 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 9 May 2023 14:11:51 -0600 Subject: [PATCH 02/10] per #2168, added unit tests to cover function that had a bug --- .../pytests/util/time_util/test_time_util.py | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/internal/tests/pytests/util/time_util/test_time_util.py b/internal/tests/pytests/util/time_util/test_time_util.py index dd7a71adff..cddf3470b3 100644 --- a/internal/tests/pytests/util/time_util/test_time_util.py +++ b/internal/tests/pytests/util/time_util/test_time_util.py @@ -123,7 +123,7 @@ def test_get_relativedelta(key, value): ) @pytest.mark.util def test_time_string_to_met_time(time_string, default_unit, met_time): - assert time_util.time_string_to_met_time(time_string, default_unit) == met_time + assert time_util.time_string_to_met_time(time_string, default_unit) == met_time @pytest.mark.parametrize( @@ -146,3 +146,44 @@ def test_ti_calculate(input_dict, expected_time_info): for key, value in expected_time_info.items(): assert time_info[key] == value assert time_info2[key] == value + + +@pytest.mark.parametrize( + 'lead, valid_time, expected_val', [ + # returns None if lead is not a relativedelta object + (None, None, None), + (1, None, None), + ('1', None, None), + # leads that can be computed without a reference time + (relativedelta(seconds=3), None, 3), + (relativedelta(minutes=3), None, 180), + (relativedelta(hours=3), None, 10800), + (relativedelta(days=3), None, 259200), + (relativedelta(minutes=3, seconds=3), None, 183), + # leads that cannot be computed without a reference time + (relativedelta(months=3), None, None), + (relativedelta(years=3), None, None), + (relativedelta(months=3, years=3), None, None), + # valid time is string 'ALL' - should behave the same as no valid time + (relativedelta(seconds=3), 'ALL', 3), + (relativedelta(minutes=3), 'ALL', 180), + (relativedelta(hours=3), 'ALL', 10800), + (relativedelta(days=3), 'ALL', 259200), + (relativedelta(minutes=3, seconds=3), 'ALL', 183), + (relativedelta(months=3), 'ALL', None), + (relativedelta(years=3), 'ALL', None), + (relativedelta(months=3, years=3), 'ALL', None), + # valid time is datetime object - months and years should work + (relativedelta(seconds=3), datetime(2014, 10, 31, 12), 3), + (relativedelta(minutes=3), datetime(2014, 10, 31, 12), 180), + (relativedelta(hours=3), datetime(2014, 10, 31, 12), 10800), + (relativedelta(days=3), datetime(2014, 10, 31, 12), 259200), + (relativedelta(minutes=3, seconds=3), datetime(2014, 10, 31, 12), 183), + (relativedelta(months=1), datetime(2014, 10, 31, 12), 2678400), + (relativedelta(years=1), datetime(2014, 10, 31, 12), 31536000), + (relativedelta(months=1, years=1), datetime(2014, 10, 31, 12), 34214400), + ] +) +@pytest.mark.util +def test_ti_get_seconds_from_relativedelta(lead, valid_time, expected_val): + assert time_util.ti_get_seconds_from_relativedelta(lead, valid_time) == expected_val From 2b109ac1211889cb8b43dcedf6803d0ba6edaebe Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 10 May 2023 09:46:09 -0600 Subject: [PATCH 03/10] fix installation of docker-compose on alpine OS that suddenly started failing --- .github/jobs/get_metviewer.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/jobs/get_metviewer.sh b/.github/jobs/get_metviewer.sh index f96752758c..f78c1a5f74 100755 --- a/.github/jobs/get_metviewer.sh +++ b/.github/jobs/get_metviewer.sh @@ -7,6 +7,7 @@ export METVIEWER_DIR=$RUNNER_WORKSPACE/output/metviewer export METVIEWER_DOCKER_IMAGE=dtcenter/metviewer:develop # install docker-compose +apk update apk add docker-compose # download docker-compose.yml file from METviewer develop branch From b9c8899ba58b9e7def86307e8620d2097ec5838a Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 10 May 2023 10:06:42 -0600 Subject: [PATCH 04/10] install docker-compose using pip because 'apk add docker-compose' no longer works --- .github/jobs/get_metviewer.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/jobs/get_metviewer.sh b/.github/jobs/get_metviewer.sh index f78c1a5f74..32445bf1cb 100755 --- a/.github/jobs/get_metviewer.sh +++ b/.github/jobs/get_metviewer.sh @@ -8,7 +8,8 @@ export METVIEWER_DOCKER_IMAGE=dtcenter/metviewer:develop # install docker-compose apk update -apk add docker-compose +apk --update add 'py-pip' +pip install 'docker-compose==1.8.0' # download docker-compose.yml file from METviewer develop branch wget https://raw.githubusercontent.com/dtcenter/METviewer/develop/docker/docker-compose.yml From 6b723db339e56ca119e84a031466514ceb8e9968 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 10 May 2023 10:32:03 -0600 Subject: [PATCH 05/10] test use case that was failing due to apk docker-compose issue --- .github/parm/use_case_groups.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/parm/use_case_groups.json b/.github/parm/use_case_groups.json index 0567a7db3e..bdc61cc702 100644 --- a/.github/parm/use_case_groups.json +++ b/.github/parm/use_case_groups.json @@ -47,7 +47,7 @@ { "category": "short_range", "index_list": "9", - "run": false + "run": true }, { "category": "short_range", From 32135a75a885efaf1a26b336d37251700db0317a Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 10 May 2023 11:06:49 -0600 Subject: [PATCH 06/10] try another approach to install docker compose --- .github/jobs/get_metviewer.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/jobs/get_metviewer.sh b/.github/jobs/get_metviewer.sh index 32445bf1cb..f83dc5da05 100755 --- a/.github/jobs/get_metviewer.sh +++ b/.github/jobs/get_metviewer.sh @@ -8,14 +8,13 @@ export METVIEWER_DOCKER_IMAGE=dtcenter/metviewer:develop # install docker-compose apk update -apk --update add 'py-pip' -pip install 'docker-compose==1.8.0' +apk add docker docker-cli-compose # download docker-compose.yml file from METviewer develop branch wget https://raw.githubusercontent.com/dtcenter/METviewer/develop/docker/docker-compose.yml # Run docker-compose to create the containers -docker-compose up -d +docker compose up -d # sleep for a few seconds to ensure database has fully started sleep 20 From e231d251128c6db84d64fe365b1f08e43fab8264 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 10 May 2023 11:13:22 -0600 Subject: [PATCH 07/10] removed install of docker because it is already done in .github/actions/run_tests/Dockerfile --- .github/jobs/get_metviewer.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/jobs/get_metviewer.sh b/.github/jobs/get_metviewer.sh index f83dc5da05..a590f92714 100755 --- a/.github/jobs/get_metviewer.sh +++ b/.github/jobs/get_metviewer.sh @@ -8,7 +8,7 @@ export METVIEWER_DOCKER_IMAGE=dtcenter/metviewer:develop # install docker-compose apk update -apk add docker docker-cli-compose +apk add docker-cli-compose # download docker-compose.yml file from METviewer develop branch wget https://raw.githubusercontent.com/dtcenter/METviewer/develop/docker/docker-compose.yml From 3f61ea8cc03a40edb4ae1dde4cc419a2b0b57862 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 10 May 2023 11:43:59 -0600 Subject: [PATCH 08/10] turn off use case after confirming fix resolved the issue with docker compose --- .github/parm/use_case_groups.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/parm/use_case_groups.json b/.github/parm/use_case_groups.json index bdc61cc702..0567a7db3e 100644 --- a/.github/parm/use_case_groups.json +++ b/.github/parm/use_case_groups.json @@ -47,7 +47,7 @@ { "category": "short_range", "index_list": "9", - "run": true + "run": false }, { "category": "short_range", From 4df71a6d7fbdf2dc028e2df0f3eda184701d0a9f Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 10 May 2023 12:39:00 -0600 Subject: [PATCH 09/10] more changes to remove SonarQube complaints --- produtil/cluster.py | 27 ++++++++++++++------------- produtil/datastore.py | 1 - produtil/fileop.py | 2 +- produtil/mpi_impl/mpi_impl_base.py | 1 - produtil/mpi_impl/srun.py | 6 +++--- produtil/prog.py | 4 ++-- 6 files changed, 20 insertions(+), 21 deletions(-) diff --git a/produtil/cluster.py b/produtil/cluster.py index 620e4ad603..469c397440 100644 --- a/produtil/cluster.py +++ b/produtil/cluster.py @@ -312,21 +312,22 @@ def production(self): file, so the runtime is likely several milliseconds when the cache times out.""" now=int(time.time()) - if self._production is None or \ - now-self._lastprod>self._prod_cache_time: - prod=False - if os.path.exists('/etc/prod'): - with open('/etc/prod','rt') as f: - for line in f: - if re.match('[a-z]+',line): - prod = line.strip()==self.name - break - self._production=prod - self._lastprod=int(time.time()) - return prod - else: + if (self._production is not None and + now-self._lastprod<=self._prod_cache_time): return self._production + prod=False + if os.path.exists('/etc/prod'): + with open('/etc/prod','rt') as f: + for line in f: + if re.match('[a-z]+',line): + prod = line.strip()==self.name + break + self._production=prod + self._lastprod=int(time.time()) + return prod + + class WCOSSCray(NOAAWCOSS): """!This subclass of NOAAWCOSS handles the new Cray portions of WCOSS: Luna and Surge.""" diff --git a/produtil/datastore.py b/produtil/datastore.py index da34117ccf..6dfbd507ec 100644 --- a/produtil/datastore.py +++ b/produtil/datastore.py @@ -1172,7 +1172,6 @@ def products(self,*args,**kwargs): class. The default implementation returns immediately without doing anything. @param args,kwargs Implementation-defined, used by subclasses.""" - return for x in []: yield x # ensures this is an iterator def log(self): """!Returns the logger object for this task.""" diff --git a/produtil/fileop.py b/produtil/fileop.py index 687bec88e4..f37c56948c 100644 --- a/produtil/fileop.py +++ b/produtil/fileop.py @@ -1080,7 +1080,7 @@ def checkfiles(self,maxwait=1800,sleeptime=20,logger=None, flogger=logger else: flogger=None - while None is None: + while True: if len(self._fset)<=0: if logger is not None: logger.info('No files to check.') diff --git a/produtil/mpi_impl/mpi_impl_base.py b/produtil/mpi_impl/mpi_impl_base.py index 701928ec51..76190090e4 100644 --- a/produtil/mpi_impl/mpi_impl_base.py +++ b/produtil/mpi_impl/mpi_impl_base.py @@ -96,7 +96,6 @@ def __init__(self,logger=None): def synonyms(): """!Iterates over alternative names for this MPI implementation, such as the names of other MPI implementations this class can handle.""" - return yield 'xyz' # trick to ensure this is an iterator def getmpiserial_path(self): diff --git a/produtil/mpi_impl/srun.py b/produtil/mpi_impl/srun.py index f2b9a54375..6130df8d04 100644 --- a/produtil/mpi_impl/srun.py +++ b/produtil/mpi_impl/srun.py @@ -115,8 +115,8 @@ def _get_available_nodes(self): status=p.poll() for line in nodelist.splitlines(): node=line.strip() - if not node: next - if node in nodeset: next + if not node: continue + if node in nodeset: continue nodeset.add(node) available_nodes.append(node) return available_nodes @@ -169,7 +169,7 @@ def mpirunner_impl(self,arg,allranks=False,rewrite_nodefile=True,**kwargs): remaining_nodes=list(available_nodes) for rank,count in arg.expand_iter(expand=False): - if count<1: next + if count<1: continue cmdfile.append('%d-%d %s'%(irank,irank+count-1,rank.to_shell())) irank+=count if rewrite_nodefile: diff --git a/produtil/prog.py b/produtil/prog.py index 839899bdb5..e8a24c2113 100644 --- a/produtil/prog.py +++ b/produtil/prog.py @@ -437,9 +437,9 @@ def __init__(self,args,**kwargs): if 'env' in kwargs: self._env=dict(kwargs['env']) # Initialize input/output/error if requested: - if 'in' in kwargs: selfkwargs['out'] + #if 'out' in kwargs: self>kwargs['out'] if 'outa' in kwargs: self>>kwargs['outa'] if 'err2out' in kwargs: self.err2out() if 'err' in kwargs: self.err(kwargs['err'],append=False) From 6291e7c45e5324f1bd6f9ce89d82cbb08ae891bb Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 10 May 2023 12:47:45 -0600 Subject: [PATCH 10/10] Revert "more changes to remove SonarQube complaints" This reverts commit 4df71a6d7fbdf2dc028e2df0f3eda184701d0a9f. --- produtil/cluster.py | 27 +++++++++++++-------------- produtil/datastore.py | 1 + produtil/fileop.py | 2 +- produtil/mpi_impl/mpi_impl_base.py | 1 + produtil/mpi_impl/srun.py | 6 +++--- produtil/prog.py | 4 ++-- 6 files changed, 21 insertions(+), 20 deletions(-) diff --git a/produtil/cluster.py b/produtil/cluster.py index 469c397440..620e4ad603 100644 --- a/produtil/cluster.py +++ b/produtil/cluster.py @@ -312,22 +312,21 @@ def production(self): file, so the runtime is likely several milliseconds when the cache times out.""" now=int(time.time()) - if (self._production is not None and - now-self._lastprod<=self._prod_cache_time): + if self._production is None or \ + now-self._lastprod>self._prod_cache_time: + prod=False + if os.path.exists('/etc/prod'): + with open('/etc/prod','rt') as f: + for line in f: + if re.match('[a-z]+',line): + prod = line.strip()==self.name + break + self._production=prod + self._lastprod=int(time.time()) + return prod + else: return self._production - prod=False - if os.path.exists('/etc/prod'): - with open('/etc/prod','rt') as f: - for line in f: - if re.match('[a-z]+',line): - prod = line.strip()==self.name - break - self._production=prod - self._lastprod=int(time.time()) - return prod - - class WCOSSCray(NOAAWCOSS): """!This subclass of NOAAWCOSS handles the new Cray portions of WCOSS: Luna and Surge.""" diff --git a/produtil/datastore.py b/produtil/datastore.py index 6dfbd507ec..da34117ccf 100644 --- a/produtil/datastore.py +++ b/produtil/datastore.py @@ -1172,6 +1172,7 @@ def products(self,*args,**kwargs): class. The default implementation returns immediately without doing anything. @param args,kwargs Implementation-defined, used by subclasses.""" + return for x in []: yield x # ensures this is an iterator def log(self): """!Returns the logger object for this task.""" diff --git a/produtil/fileop.py b/produtil/fileop.py index f37c56948c..687bec88e4 100644 --- a/produtil/fileop.py +++ b/produtil/fileop.py @@ -1080,7 +1080,7 @@ def checkfiles(self,maxwait=1800,sleeptime=20,logger=None, flogger=logger else: flogger=None - while True: + while None is None: if len(self._fset)<=0: if logger is not None: logger.info('No files to check.') diff --git a/produtil/mpi_impl/mpi_impl_base.py b/produtil/mpi_impl/mpi_impl_base.py index 76190090e4..701928ec51 100644 --- a/produtil/mpi_impl/mpi_impl_base.py +++ b/produtil/mpi_impl/mpi_impl_base.py @@ -96,6 +96,7 @@ def __init__(self,logger=None): def synonyms(): """!Iterates over alternative names for this MPI implementation, such as the names of other MPI implementations this class can handle.""" + return yield 'xyz' # trick to ensure this is an iterator def getmpiserial_path(self): diff --git a/produtil/mpi_impl/srun.py b/produtil/mpi_impl/srun.py index 6130df8d04..f2b9a54375 100644 --- a/produtil/mpi_impl/srun.py +++ b/produtil/mpi_impl/srun.py @@ -115,8 +115,8 @@ def _get_available_nodes(self): status=p.poll() for line in nodelist.splitlines(): node=line.strip() - if not node: continue - if node in nodeset: continue + if not node: next + if node in nodeset: next nodeset.add(node) available_nodes.append(node) return available_nodes @@ -169,7 +169,7 @@ def mpirunner_impl(self,arg,allranks=False,rewrite_nodefile=True,**kwargs): remaining_nodes=list(available_nodes) for rank,count in arg.expand_iter(expand=False): - if count<1: continue + if count<1: next cmdfile.append('%d-%d %s'%(irank,irank+count-1,rank.to_shell())) irank+=count if rewrite_nodefile: diff --git a/produtil/prog.py b/produtil/prog.py index e8a24c2113..839899bdb5 100644 --- a/produtil/prog.py +++ b/produtil/prog.py @@ -437,9 +437,9 @@ def __init__(self,args,**kwargs): if 'env' in kwargs: self._env=dict(kwargs['env']) # Initialize input/output/error if requested: - #if 'in' in kwargs: selfkwargs['out'] + if 'out' in kwargs: self>kwargs['out'] if 'outa' in kwargs: self>>kwargs['outa'] if 'err2out' in kwargs: self.err2out() if 'err' in kwargs: self.err(kwargs['err'],append=False)