From 2ac9ea36d1af1abbf6804fc65a86728417cce605 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 2 Dec 2019 12:50:07 +0000 Subject: [PATCH 1/7] template for forward and reverse lookup --- cylc/flow/platform_lookup.py | 65 +++++++++++++++++++++++++ cylc/flow/tests/test_platform_lookup.py | 35 +++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 cylc/flow/platform_lookup.py create mode 100644 cylc/flow/tests/test_platform_lookup.py diff --git a/cylc/flow/platform_lookup.py b/cylc/flow/platform_lookup.py new file mode 100644 index 00000000000..3ee8ab8a9a8 --- /dev/null +++ b/cylc/flow/platform_lookup.py @@ -0,0 +1,65 @@ +# THIS FILE IS PART OF THE CYLC SUITE ENGINE. +# Copyright (C) 2008-2019 NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# Tests for the platform lookup. + +import re + +def forward_lookup(task_platform, platforms): + """ + Find out which job platform to use given a list of possible platforms and + a task platform string. + + Args: + task_platform (str): + platform item from config [runtime][TASK][platform] + platforms (list): + list of possible platforms defined by global.rc + + Returns: + platform (str): + string representing a platform from the global config. + + Examples: + Mel - write some doctests here... + """ + platform = "to be implemented" + return platform + + +def reverse_lookup(task_job, task_remote, platforms): + """ + Find out which job platform to use given a list of possible platforms + and the task dictionary with cylc 7 definitions in it. + + Args: + task_job (dict): + Suite config [runtime][TASK][job] section + task_remote (dict): + Suite config [runtime][TASK][remote] section + platforms (dict): + Dictionary containing platfrom definitions. + + Returns: + platfrom (str): + string representing a platform from the global config. + + Examples: + Tim - write some doctests here... + + """ + platform = "to be implemented" + return platform \ No newline at end of file diff --git a/cylc/flow/tests/test_platform_lookup.py b/cylc/flow/tests/test_platform_lookup.py new file mode 100644 index 00000000000..a0ee602777e --- /dev/null +++ b/cylc/flow/tests/test_platform_lookup.py @@ -0,0 +1,35 @@ +# THIS FILE IS PART OF THE CYLC SUITE ENGINE. +# Copyright (C) 2008-2019 NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# Tests for the platform lookup. + +from cylc.flow.platform_lookup import forward_lookup, reverse_lookup + + +class TestForwardLookup(): + """ + Tests to ensure that the job platform forward lookup works as intended. + """ + def test_basic(self): + assert 1 == 1 + + +class TestReversLookup(): + """ + Tests to ensure that job platform reverse lookup works as intended. + """ + def test_basic(self): + assert 'Hello' == 'Hello' \ No newline at end of file From 91384cbd52c5b5b0dc9ede5c536b57837a66e79b Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 2 Dec 2019 14:08:50 +0000 Subject: [PATCH 2/7] added sample platforms dict --- cylc/flow/tests/test_platform_lookup.py | 41 +++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/cylc/flow/tests/test_platform_lookup.py b/cylc/flow/tests/test_platform_lookup.py index a0ee602777e..86769cdaa80 100644 --- a/cylc/flow/tests/test_platform_lookup.py +++ b/cylc/flow/tests/test_platform_lookup.py @@ -18,6 +18,46 @@ from cylc.flow.platform_lookup import forward_lookup, reverse_lookup +# The platforms list for testing is set as a constant +# [platforms] +# [[desktop\d\d|laptop\d\d]] +# # hosts = platform name (default) +# # Note: "desktop01" and "desktop02" are both valid and distinct platforms +# [[sugar]] +# hosts = localhost +# batch system = slurm +# [[hpc]] +# hosts = hpcl1, hpcl2 +# retrieve job logs = True +# batch system = pbs +# [[hpcl1-bg]] +# hosts = hpcl1 +# retrieve job logs = True +# batch system = background +# [[hpcl2-bg]] +# hosts = hpcl2 +# retrieve job logs = True +# batch system = background +PLATFORMS = { + 'desktop\d\d|laptop\d\d': None, + 'sugar': { + 'hosts': 'localhost', + 'batch system': 'slurm', + }, + 'hpc': { + 'hosts': ['hpc1', 'hpc2'], + 'batch system': 'pbs', + }, + 'hpc1-bg': { + 'hosts': 'hpc1', + 'batch system': 'background', + }, + 'hpc2-bg': { + 'hosts': 'hpc2', + 'batch system': 'background' + } +} + class TestForwardLookup(): """ @@ -31,5 +71,6 @@ class TestReversLookup(): """ Tests to ensure that job platform reverse lookup works as intended. """ + def test_basic(self): assert 'Hello' == 'Hello' \ No newline at end of file From 6ccbed088ca2c61c07f7f840ce6a6f1f86e0cf1b Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 3 Dec 2019 12:31:13 +0000 Subject: [PATCH 3/7] more infrastructure --- cylc/flow/exceptions.py | 5 ++++ cylc/flow/platform_lookup.py | 7 +++--- cylc/flow/tests/test_platform_lookup.py | 33 +++++++++++++++++++------ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/cylc/flow/exceptions.py b/cylc/flow/exceptions.py index 06c5bfcb15a..2a05dfb85ee 100644 --- a/cylc/flow/exceptions.py +++ b/cylc/flow/exceptions.py @@ -167,3 +167,8 @@ class CylcMissingContextPointError(CyclingError): class CylcMissingFinalCyclePointError(CyclingError): """An error denoting a missing (but required) final cycle point.""" + + +class PlatformLookupError(CylcConfigError): + """Unable to determine the correct job platform from the information + given""" diff --git a/cylc/flow/platform_lookup.py b/cylc/flow/platform_lookup.py index 3ee8ab8a9a8..da4eb4edb33 100644 --- a/cylc/flow/platform_lookup.py +++ b/cylc/flow/platform_lookup.py @@ -17,6 +17,7 @@ # Tests for the platform lookup. import re +from cylc.flow.exceptions import PlatformLookupError def forward_lookup(task_platform, platforms): """ @@ -36,8 +37,7 @@ def forward_lookup(task_platform, platforms): Examples: Mel - write some doctests here... """ - platform = "to be implemented" - return platform + raise NotImplementedError def reverse_lookup(task_job, task_remote, platforms): @@ -61,5 +61,4 @@ def reverse_lookup(task_job, task_remote, platforms): Tim - write some doctests here... """ - platform = "to be implemented" - return platform \ No newline at end of file + raise NotImplementedError \ No newline at end of file diff --git a/cylc/flow/tests/test_platform_lookup.py b/cylc/flow/tests/test_platform_lookup.py index 86769cdaa80..19168acb97a 100644 --- a/cylc/flow/tests/test_platform_lookup.py +++ b/cylc/flow/tests/test_platform_lookup.py @@ -16,7 +16,9 @@ # # Tests for the platform lookup. +import pytest from cylc.flow.platform_lookup import forward_lookup, reverse_lookup +from cylc.flow.exceptions import PlatformLookupError # The platforms list for testing is set as a constant # [platforms] @@ -39,25 +41,43 @@ # retrieve job logs = True # batch system = background PLATFORMS = { + 'suite server platform': None, 'desktop\d\d|laptop\d\d': None, 'sugar': { - 'hosts': 'localhost', + 'login hosts': 'localhost', 'batch system': 'slurm', }, 'hpc': { - 'hosts': ['hpc1', 'hpc2'], + 'login hosts': ['hpc1', 'hpc2'], 'batch system': 'pbs', }, 'hpc1-bg': { - 'hosts': 'hpc1', + 'login hosts': 'hpc1', 'batch system': 'background', }, 'hpc2-bg': { - 'hosts': 'hpc2', + 'login hosts': 'hpc2', 'batch system': 'background' } } +PLATFORMS_NO_UNIQUE = { + 'sugar': { + 'login hosts': 'localhost', + 'batch system': 'slurm', + }, + 'pepper': { + 'login hosts': ['hpc1', 'hpc2'], + 'batch system': 'slurm', + }, + +} + + +PLATFORMS_WITH_RE = { + # Mel - make up some amusing platforms doing wierd stuff with regexes +} + class TestForwardLookup(): """ @@ -67,10 +87,7 @@ def test_basic(self): assert 1 == 1 -class TestReversLookup(): +class TestReverseLookup(): """ Tests to ensure that job platform reverse lookup works as intended. """ - - def test_basic(self): - assert 'Hello' == 'Hello' \ No newline at end of file From 44bf50cf8b4812ea3bbc56c8590692411d01f7d2 Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Thu, 5 Dec 2019 09:19:24 +0000 Subject: [PATCH 4/7] Adding forward lookup and related tests. --- cylc/flow/platform_lookup.py | 49 +++++++++++++++++++--- cylc/flow/tests/test_platform_lookup.py | 56 ++++++++++++++++--------- 2 files changed, 79 insertions(+), 26 deletions(-) diff --git a/cylc/flow/platform_lookup.py b/cylc/flow/platform_lookup.py index da4eb4edb33..c4768a34561 100644 --- a/cylc/flow/platform_lookup.py +++ b/cylc/flow/platform_lookup.py @@ -19,25 +19,62 @@ import re from cylc.flow.exceptions import PlatformLookupError -def forward_lookup(task_platform, platforms): + +def forward_lookup(platforms, task_platform): """ Find out which job platform to use given a list of possible platforms and a task platform string. + Verifies selected platform is present in global.rc file and returns it, + raises error if platfrom is not in global.rc or returns 'localhost' if + no platform is initally selected. + Args: task_platform (str): platform item from config [runtime][TASK][platform] - platforms (list): + platforms (dictionary): list of possible platforms defined by global.rc Returns: platform (str): string representing a platform from the global config. - Examples: - Mel - write some doctests here... + Example: + Example Input: + platforms = { + 'suite server platform': None, + 'desktop[0-9][0-9]|laptop[0-9][0-9]': None, + 'sugar': { + 'login hosts': 'localhost', + 'batch system': 'slurm' + }, + 'hpc': { + 'login hosts': ['hpc1', 'hpc2'], + 'batch system': 'pbs' + }, + 'hpc1-bg': { + 'login hosts': 'hpc1', + 'batch system': 'background' + }, + 'hpc2-bg': { + 'login hosts': 'hpc2', + 'batch system': 'background' + } + } + task_platform = desktop22 + + Example Output: desktop22 """ - raise NotImplementedError + if task_platform is None: + return 'localhost' + platforms = list(platforms.keys()) + reversed_platforms = platforms[::-1] + for platform in reversed_platforms: + if re.fullmatch(platform, task_platform): + return task_platform + + raise PlatformLookupError( + f"No matching platform \"{task_platform}\" found") def reverse_lookup(task_job, task_remote, platforms): @@ -61,4 +98,4 @@ def reverse_lookup(task_job, task_remote, platforms): Tim - write some doctests here... """ - raise NotImplementedError \ No newline at end of file + raise NotImplementedError diff --git a/cylc/flow/tests/test_platform_lookup.py b/cylc/flow/tests/test_platform_lookup.py index 19168acb97a..006f09c0160 100644 --- a/cylc/flow/tests/test_platform_lookup.py +++ b/cylc/flow/tests/test_platform_lookup.py @@ -24,7 +24,8 @@ # [platforms] # [[desktop\d\d|laptop\d\d]] # # hosts = platform name (default) -# # Note: "desktop01" and "desktop02" are both valid and distinct platforms +# # Note: "desktop01" and "desktop02" are both valid and distinct +# platforms # [[sugar]] # hosts = localhost # batch system = slurm @@ -40,20 +41,21 @@ # hosts = hpcl2 # retrieve job logs = True # batch system = background -PLATFORMS = { + +PLATFORMS_STANDARD = { 'suite server platform': None, - 'desktop\d\d|laptop\d\d': None, + 'desktop[0-9][0-9]|laptop[0-9][0-9]': None, 'sugar': { 'login hosts': 'localhost', - 'batch system': 'slurm', + 'batch system': 'slurm' }, 'hpc': { 'login hosts': ['hpc1', 'hpc2'], - 'batch system': 'pbs', + 'batch system': 'pbs' }, 'hpc1-bg': { 'login hosts': 'hpc1', - 'batch system': 'background', + 'batch system': 'background' }, 'hpc2-bg': { 'login hosts': 'hpc2', @@ -64,30 +66,44 @@ PLATFORMS_NO_UNIQUE = { 'sugar': { 'login hosts': 'localhost', - 'batch system': 'slurm', + 'batch system': 'slurm' }, 'pepper': { 'login hosts': ['hpc1', 'hpc2'], - 'batch system': 'slurm', + 'batch system': 'slurm' }, } - PLATFORMS_WITH_RE = { - # Mel - make up some amusing platforms doing wierd stuff with regexes + 'hpc.*': {'login hosts': 'hpc1', 'batch system': 'background'}, + 'h.*': {'login hosts': 'hpc3'}, + r'vld\d{3}|vld\d{2}': None, + 'nu.*': {'batch system': 'slurm'} } -class TestForwardLookup(): - """ - Tests to ensure that the job platform forward lookup works as intended. - """ - def test_basic(self): - assert 1 == 1 +@pytest.mark.parametrize( + "PLATFORMS, platform, expected", + [(PLATFORMS_WITH_RE, 'nutmeg', 'nutmeg'), + (PLATFORMS_WITH_RE, 'vld798', 'vld798'), + (PLATFORMS_WITH_RE, 'vld56', 'vld56'), + (PLATFORMS_NO_UNIQUE, 'sugar', 'sugar'), + (PLATFORMS_STANDARD, None, 'localhost'), + (PLATFORMS_STANDARD, 'laptop22', 'laptop22'), + (PLATFORMS_STANDARD, 'hpc1-bg', 'hpc1-bg'), + (PLATFORMS_WITH_RE, 'hpc2', 'hpc2') + ] +) +def test_basic(PLATFORMS, platform, expected): + assert forward_lookup(PLATFORMS, platform) == expected + + +def test_platform_not_there(): + with pytest.raises(PlatformLookupError): + forward_lookup(PLATFORMS_STANDARD, 'moooo') -class TestReverseLookup(): - """ - Tests to ensure that job platform reverse lookup works as intended. - """ +def test_similar_but_not_exact_match(): + with pytest.raises(PlatformLookupError): + forward_lookup(PLATFORMS_WITH_RE, 'vld1') From ce0d0ecd6fc8b7f23f5ad62e2cb086d680b6fc46 Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Fri, 6 Dec 2019 09:45:35 +0000 Subject: [PATCH 5/7] Changed example to doctest. --- cylc/flow/platform_lookup.py | 46 ++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/cylc/flow/platform_lookup.py b/cylc/flow/platform_lookup.py index c4768a34561..9b1c12f51f7 100644 --- a/cylc/flow/platform_lookup.py +++ b/cylc/flow/platform_lookup.py @@ -41,29 +41,29 @@ def forward_lookup(platforms, task_platform): Example: Example Input: - platforms = { - 'suite server platform': None, - 'desktop[0-9][0-9]|laptop[0-9][0-9]': None, - 'sugar': { - 'login hosts': 'localhost', - 'batch system': 'slurm' - }, - 'hpc': { - 'login hosts': ['hpc1', 'hpc2'], - 'batch system': 'pbs' - }, - 'hpc1-bg': { - 'login hosts': 'hpc1', - 'batch system': 'background' - }, - 'hpc2-bg': { - 'login hosts': 'hpc2', - 'batch system': 'background' - } - } - task_platform = desktop22 - - Example Output: desktop22 + >>> platforms = { + ... 'suite server platform': None, + ... 'desktop[0-9][0-9]|laptop[0-9][0-9]': None, + ... 'sugar': { + ... 'login hosts': 'localhost', + ... 'batch system': 'slurm' + ... }, + ... 'hpc': { + ... 'login hosts': ['hpc1', 'hpc2'], + ... 'batch system': 'pbs' + ... }, + ... 'hpc1-bg': { + ... 'login hosts': 'hpc1', + ... 'batch system': 'background' + ... }, + ... 'hpc2-bg': { + ... 'login hosts': 'hpc2', + ... 'batch system': 'background' + ... } + ... } + >>> task_platform = 'desktop22' + >>> forward_lookup(platforms, task_platform) + 'desktop22' """ if task_platform is None: return 'localhost' From 46136d1c3a4d5755b748a5313e37a899dc79840c Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Tue, 10 Dec 2019 15:10:00 +0000 Subject: [PATCH 6/7] Changes to regex as advised by Oliver. --- cylc/flow/tests/test_platform_lookup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/tests/test_platform_lookup.py b/cylc/flow/tests/test_platform_lookup.py index 006f09c0160..25d5ad69295 100644 --- a/cylc/flow/tests/test_platform_lookup.py +++ b/cylc/flow/tests/test_platform_lookup.py @@ -78,7 +78,7 @@ PLATFORMS_WITH_RE = { 'hpc.*': {'login hosts': 'hpc1', 'batch system': 'background'}, 'h.*': {'login hosts': 'hpc3'}, - r'vld\d{3}|vld\d{2}': None, + r'vld\d{2,3}': None, 'nu.*': {'batch system': 'slurm'} } From 779a02ace24073db33b0f946e59f5016d4228566 Mon Sep 17 00:00:00 2001 From: Mel Hall <37735232+datamel@users.noreply.github.com> Date: Wed, 11 Dec 2019 13:00:05 +0000 Subject: [PATCH 7/7] Small changes made after review - changed slicing reverse to reversed - changed task_platform to job_platform - removed development comment --- cylc/flow/platform_lookup.py | 23 ++++++++++------------- cylc/flow/tests/test_platform_lookup.py | 22 ---------------------- 2 files changed, 10 insertions(+), 35 deletions(-) diff --git a/cylc/flow/platform_lookup.py b/cylc/flow/platform_lookup.py index 9b1c12f51f7..268904e03ce 100644 --- a/cylc/flow/platform_lookup.py +++ b/cylc/flow/platform_lookup.py @@ -20,7 +20,7 @@ from cylc.flow.exceptions import PlatformLookupError -def forward_lookup(platforms, task_platform): +def forward_lookup(platforms, job_platform): """ Find out which job platform to use given a list of possible platforms and a task platform string. @@ -30,8 +30,8 @@ def forward_lookup(platforms, task_platform): no platform is initally selected. Args: - task_platform (str): - platform item from config [runtime][TASK][platform] + job_platform (str): + platform item from config [runtime][TASK]platform platforms (dictionary): list of possible platforms defined by global.rc @@ -40,7 +40,6 @@ def forward_lookup(platforms, task_platform): string representing a platform from the global config. Example: - Example Input: >>> platforms = { ... 'suite server platform': None, ... 'desktop[0-9][0-9]|laptop[0-9][0-9]': None, @@ -61,20 +60,18 @@ def forward_lookup(platforms, task_platform): ... 'batch system': 'background' ... } ... } - >>> task_platform = 'desktop22' - >>> forward_lookup(platforms, task_platform) + >>> job_platform = 'desktop22' + >>> forward_lookup(platforms, job_platform) 'desktop22' """ - if task_platform is None: + if job_platform is None: return 'localhost' - platforms = list(platforms.keys()) - reversed_platforms = platforms[::-1] - for platform in reversed_platforms: - if re.fullmatch(platform, task_platform): - return task_platform + for platform in reversed(list(platforms)): + if re.fullmatch(platform, job_platform): + return job_platform raise PlatformLookupError( - f"No matching platform \"{task_platform}\" found") + f"No matching platform \"{job_platform}\" found") def reverse_lookup(task_job, task_remote, platforms): diff --git a/cylc/flow/tests/test_platform_lookup.py b/cylc/flow/tests/test_platform_lookup.py index 25d5ad69295..064c3cceef7 100644 --- a/cylc/flow/tests/test_platform_lookup.py +++ b/cylc/flow/tests/test_platform_lookup.py @@ -20,28 +20,6 @@ from cylc.flow.platform_lookup import forward_lookup, reverse_lookup from cylc.flow.exceptions import PlatformLookupError -# The platforms list for testing is set as a constant -# [platforms] -# [[desktop\d\d|laptop\d\d]] -# # hosts = platform name (default) -# # Note: "desktop01" and "desktop02" are both valid and distinct -# platforms -# [[sugar]] -# hosts = localhost -# batch system = slurm -# [[hpc]] -# hosts = hpcl1, hpcl2 -# retrieve job logs = True -# batch system = pbs -# [[hpcl1-bg]] -# hosts = hpcl1 -# retrieve job logs = True -# batch system = background -# [[hpcl2-bg]] -# hosts = hpcl2 -# retrieve job logs = True -# batch system = background - PLATFORMS_STANDARD = { 'suite server platform': None, 'desktop[0-9][0-9]|laptop[0-9][0-9]': None,