From 1a6a04211758f7794b99fbf720bed68a8c4fdb7b Mon Sep 17 00:00:00 2001 From: william cross Date: Fri, 26 Jul 2024 14:05:08 +0100 Subject: [PATCH 1/4] Added extra checks for fqan fields In record.py added extra checks to prevent None type being used for fqan fields as Null should not be in the database for them Added extra tests to ensure correct datatypes where used --- apel/db/records/record.py | 4 +++- test/test_blah.py | 21 +++++++++++++++++++++ test/test_parsing_utils.py | 4 ++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/apel/db/records/record.py b/apel/db/records/record.py index 834b81b2b..fb95aaa18 100644 --- a/apel/db/records/record.py +++ b/apel/db/records/record.py @@ -79,6 +79,8 @@ def __init__(self): self._datetime_fields = [] # The dictionary into which all the information goes self._record_content = {} + # These fields need special handling as they should never insert Null into the database + self._fqan_fields = ["VO", "VOGroup", "VORole"] def set_all(self, fielddict): ''' @@ -127,7 +129,7 @@ def checked(self, name, value): ''' try: # Convert any null equivalents to a None object - if check_for_null(value): + if name not in self._fqan_fields and check_for_null(value): value = None # firstly we must ensure that we do not put None # in mandatory field diff --git a/test/test_blah.py b/test/test_blah.py index 0a10bb581..54a10600a 100644 --- a/test/test_blah.py +++ b/test/test_blah.py @@ -28,6 +28,26 @@ def test_parse(self): line1_values = {"TimeStamp": datetime.datetime(2012, 5, 20, 23, 59, 47), "GlobalUserName":"/O=GermanGrid/OU=UniWuppertal/CN=Torsten Harenberg", "FQAN": "/atlas/Role=production/Capability=NULL", + "VO": "atlas", + "VOGroup": "/atlas", + "VORole": "Role=production", + "CE": "cream-2-fzk.gridka.de:8443/cream-pbs-atlasXL", + "GlobalJobId": "CREAM410741480", + "LrmsId": "9575064.lrms1", + } + + line2 = ('"timestamp=2012-05-20 23:59:47" ' + +'"userDN=/O=GermanGrid/OU=UniWuppertal/CN=Torsten Harenberg" ' + +'"userFQAN=/wlcg" ' + +'"ceID=cream-2-fzk.gridka.de:8443/cream-pbs-atlasXL" ' + +'"jobID=CREAM410741480" "lrmsID=9575064.lrms1" "localUser=11999"') + + line2_values = {"TimeStamp": datetime.datetime(2012, 5, 20, 23, 59, 47), + "GlobalUserName":"/O=GermanGrid/OU=UniWuppertal/CN=Torsten Harenberg", + "FQAN": "/wlcg", + "VO": "wlcg", + "VOGroup": "/wlcg", + "VORole": "None", "CE": "cream-2-fzk.gridka.de:8443/cream-pbs-atlasXL", "GlobalJobId": "CREAM410741480", "LrmsId": "9575064.lrms1", @@ -35,6 +55,7 @@ def test_parse(self): cases = {} cases[line1] = line1_values + cases[line2] = line2_values for line in list(cases.keys()): diff --git a/test/test_parsing_utils.py b/test/test_parsing_utils.py index 60e30539c..9015afbb6 100644 --- a/test/test_parsing_utils.py +++ b/test/test_parsing_utils.py @@ -19,6 +19,8 @@ def test_parse_fqan(self): right_fqan2 = "/atlas/somethingelse/role=doer" right_fqan3 = "/atlas/Role=production/Capability=NULL;/atlas/Role=NULL/Capability=NULL;/atlas/alarm/Role=NULL/Capability=NULL;/atlas/au/Role=NULL/Capability=NULL;/atlas/dataprep/Role=NULL/Capability=NULL;/atlas/lcg1/Role=NULL/Capability=NULL;/atlas/team/Role=NULL/Capability=NULL;/atlas/uk/Role=NULL/Capability=NULL" right_fqan4 = "/ilc/Role=NULL/Capability=NULL" + right_fqan5 = "/wlcg" + right_fqan6 = "/wlcg/Role=pilot/Capability=NULL" short_fqan1 = "/no/role/or/equals" short_fqan2 = "/someorg/somegroup" @@ -30,6 +32,8 @@ def test_parse_fqan(self): self.assertEqual(parse_fqan(right_fqan2), ("role=doer", "/atlas/somethingelse", "atlas")) self.assertEqual(parse_fqan(right_fqan3), ("Role=production", "/atlas", "atlas")) self.assertEqual(parse_fqan(right_fqan4), ("Role=NULL", "/ilc", "ilc")) + self.assertEqual(parse_fqan(right_fqan5), ("None", "/wlcg", "wlcg")) + self.assertEqual(parse_fqan(right_fqan6), ("Role=pilot", "/wlcg", "wlcg")) self.assertEqual(parse_fqan(short_fqan1), ('None', '/no/role/or/equals', 'no')) self.assertEqual(parse_fqan(short_fqan2), ('None', '/someorg/somegroup', 'someorg')) From 0ec47e89a185a14e3a6a90d23d89b72642a09160 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Wed, 21 Aug 2024 11:10:21 +0100 Subject: [PATCH 2/4] Remove redundant code/comment This code has been in the codebase since the beginning. It hinted that special handling was needed for these fields, but nothing was ever implemented. Now we have working code in the parent Record class so this is completely redundant. --- apel/db/records/job.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/apel/db/records/job.py b/apel/db/records/job.py index 25f8febc5..fa17d1ce7 100644 --- a/apel/db/records/job.py +++ b/apel/db/records/job.py @@ -60,9 +60,6 @@ def __init__(self): "NodeCount", "StartTime", "EndTime", "InfrastructureDescription", "InfrastructureType", "MemoryReal", "MemoryVirtual", "ServiceLevelType", "ServiceLevel"] - # Not used in the message format, but required by the database. - # self._fqan_fields = ["VO", "VOGroup", "VORole"] - # Fields which are accepted but currently ignored. self._ignored_fields = ["SubmitHostType", "UpdateTime"] From d2f2e922ffe6de600fb812b2c9cf9ad9804e0752 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Wed, 21 Aug 2024 11:25:51 +0100 Subject: [PATCH 3/4] Add exception to comment to match new code --- apel/db/records/record.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apel/db/records/record.py b/apel/db/records/record.py index fb95aaa18..54adcae43 100644 --- a/apel/db/records/record.py +++ b/apel/db/records/record.py @@ -128,7 +128,7 @@ def checked(self, name, value): Otherwise it raises an error. ''' try: - # Convert any null equivalents to a None object + # Convert null equivalents (except VOMS attributes) to None object if name not in self._fqan_fields and check_for_null(value): value = None # firstly we must ensure that we do not put None From f2f38f0ddddd00b9c2e689bc2295d4938ac35026 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Wed, 21 Aug 2024 11:42:42 +0100 Subject: [PATCH 4/4] Spread out long comment line --- apel/db/records/record.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apel/db/records/record.py b/apel/db/records/record.py index 54adcae43..a0055ba3b 100644 --- a/apel/db/records/record.py +++ b/apel/db/records/record.py @@ -79,7 +79,8 @@ def __init__(self): self._datetime_fields = [] # The dictionary into which all the information goes self._record_content = {} - # These fields need special handling as they should never insert Null into the database + # These fields need special handling as they shouldn't be inserted as + # Null into the database self._fqan_fields = ["VO", "VOGroup", "VORole"] def set_all(self, fielddict):