FIX: Run cloud examples without the on-premise engine, report why a property has no value, and make a broken example go red

.github/workflows/nightly-pipeline.yml

@@ -33,7 +33,7 @@ jobs:
     secrets:
       token: ${{ secrets.ACCESS_TOKEN }}
       DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }}
-      TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY}}
+      TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_SUPER || secrets.SUPER_RESOURCE_KEY }}
       DeviceDetectionUrl: ${{ secrets.DEVICE_DETECTION_URL }}
       CsvUrl: ${{ secrets.CSV_URL }}
 
@@ -50,7 +50,7 @@ jobs:
     secrets:
       token: ${{ secrets.ACCESS_TOKEN }}
       DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }}
-      TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY}}
+      TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_SUPER || secrets.SUPER_RESOURCE_KEY }}
       DeviceDetectionUrl: ${{ secrets.DEVICE_DETECTION_URL }}
       CsvUrl: ${{ secrets.CSV_URL }}
       PypiToken: ${{ secrets.PYPI_TOKEN }}

.github/workflows/publish.yml

@@ -19,7 +19,7 @@ jobs:
     secrets:
       token: ${{ secrets.ACCESS_TOKEN }}
       DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }}
-      TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY}}
+      TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_SUPER || secrets.SUPER_RESOURCE_KEY }}
       DeviceDetectionUrl: ${{ secrets.DEVICE_DETECTION_URL }}
       CsvUrl: ${{ secrets.CSV_URL }}
       PypiToken: ${{ secrets.PYPI_TOKEN }}

.github/workflows/pull-requests.yml

@@ -19,6 +19,6 @@ jobs:
     secrets:
       token: ${{ secrets.ACCESS_TOKEN }}
       DeviceDetection: ${{ secrets.DEVICE_DETECTION_KEY }}
-      TestResourceKey: ${{ secrets.SUPER_RESOURCE_KEY}}
+      TestResourceKey: ${{ secrets._51DEGREES_RESOURCE_KEY_SUPER || secrets.SUPER_RESOURCE_KEY }}
       DeviceDetectionUrl: ${{ secrets.DEVICE_DETECTION_URL }}
       CsvUrl: ${{ secrets.CSV_URL }}

ci/run-integration-tests.ps1

@@ -29,6 +29,12 @@ if ($env:GITHUB_JOB -eq "Test") {
     pip install $RepoName/fiftyone_devicedetection_examples || $(throw "pip install failed")
 }
 
+# Resource key environment variables follow the 51Degrees convention, which
+# is that every one of them starts with '_51DEGREES_RESOURCE_KEY'. The name
+# used before the convention was adopted is set by common-ci and is still
+# read as a fallback, so anything not yet moved over keeps working.
+$env:_51DEGREES_RESOURCE_KEY = $Keys.TestResourceKey
+
 ./python/run-integration-tests.ps1 -RepoName $RepoName -Packages $packages -Keys $Keys
 $status = $LASTEXITCODE
 
@@ -41,7 +47,7 @@ try {
         $py = $IsWindows ? ".venv/Scripts/python.exe" : ".venv/bin/python"
         & $py -m pip install -e .
         $env:PORT = 8097
-        $env:resource_key = $Keys.TestResourceKey
+        $env:_51DEGREES_RESOURCE_KEY = $Keys.TestResourceKey
         $env:cloud_endpoint = "https://cloud.51degrees.com/api/v4/"
         $example = & $py -m fiftyone_devicedetection_examples.cloud.gettingstarted_web 2>&1 &
     } finally { Pop-Location }

ci/run-unit-tests.ps1

@@ -9,7 +9,11 @@ if ($IsWindows) {
     Write-Output $env:TEMP
 }
 
-$packages = "fiftyone_devicedetection_onpremise"
+# The cloud package's tests run against fixed cloud responses, so they need
+# no resource key, no licence key and no network connection. Running them
+# here means a broken cloud example shows as red on every build, rather than
+# only when the integration tests happen to have a key to run with.
+$packages = "fiftyone_devicedetection_cloud", "fiftyone_devicedetection_onpremise"
 ./python/run-unit-tests.ps1 -RepoName $RepoName -Packages $packages
 
 exit $LASTEXITCODE

fiftyone_devicedetection_cloud/src/fiftyone_devicedetection_cloud/hardwareprofile_cloud.py

@@ -28,6 +28,40 @@
 
 import json
 
+# The suffix the cloud service adds to a property name to carry the reason
+# that property has no value, for example "hardwarevendornullreason".
+NULL_REASON_SUFFIX = "nullreason"
+
+
+def _build_aspect_values(hardware):
+    """!
+    Build the aspect level property values from the "hardware" section of a
+    cloud response, pairing each null value with the reason the service gave
+    for it.
+
+    @type hardware: dict
+    @param hardware: the "hardware" section of the cloud response
+
+    @rtype dict
+    @return property name to AspectPropertyValue
+    """
+
+    values = {}
+
+    for key, value in hardware.items():
+        if key == "profiles" or key.endswith(NULL_REASON_SUFFIX):
+            continue
+
+        if value is None:
+            reason = hardware.get(key + NULL_REASON_SUFFIX)
+            values[key] = AspectPropertyValue(
+                no_value_message=reason if isinstance(reason, str) else None)
+        else:
+            values[key] = AspectPropertyValue(value=value)
+
+    return values
+
+
 class HardwareProfileCloud(CloudEngine):
     """!
     The hardware profile cloud engine
@@ -44,18 +78,40 @@ def process_internal(self, flowdata):
 
         cloud_data = json.loads(cloud_data)
 
-        # Loop over cloud_data.devices properties to check if they have a value
+        hardware = cloud_data.get("hardware") or {}
+
+        if not isinstance(hardware, dict):
+            hardware = {}
+
+        # Properties the resource key is not entitled to are returned by the
+        # cloud service at the aspect level rather than inside each profile,
+        # with a companion "<name>nullreason" saying why there is no value.
+        # Collect those so the reason can travel with every profile instead
+        # of being thrown away.
+        aspect_values = _build_aspect_values(hardware)
 
         devices = []
 
-        for profile in cloud_data["hardware"]["profiles"]:
+        for profile in hardware.get("profiles") or []:
             device = {}
             for property_key, property_value in profile.items():
                 device[property_key] = AspectPropertyValue(value=property_value)
 
+            # Add the properties the service could not supply, carrying the
+            # reason it gave, so a caller reading a profile gets an
+            # explanation rather than nothing at all.
+            for property_key, aspect_value in aspect_values.items():
+                if property_key not in device:
+                    device[property_key] = aspect_value
+
             devices.append(device)
 
-        data = AspectDataDictionary(self, {"profiles": devices})
+        # The aspect level values are exposed alongside the profiles so a
+        # caller with no matching profiles can still read the reason.
+        contents = dict(aspect_values)
+        contents["profiles"] = devices
+
+        data = AspectDataDictionary(self, contents)
 
         flowdata.set_element_data(data)
 

fiftyone_devicedetection_cloud/tests/test_cloud.py

@@ -29,24 +29,49 @@
 
 from fiftyone_devicedetection_cloud.devicedetection_cloud import DeviceDetectionCloud
 
-if not "resource_key" in os.environ:
-    print("To run the cloud tests, please set a valid 51Degrees cloud resource key as the resource_key environment variable. e.g `export resource_key=MYresource_key` on the command line")
+# Resource key environment variables follow the 51Degrees convention, which
+# is that every one of them starts with "_51DEGREES_RESOURCE_KEY". The name
+# used before the convention was adopted is still read, so an existing setup
+# keeps working.
+RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY"
+LEGACY_RESOURCE_KEY_ENV_VAR = "resource_key"
+
+MISSING_KEY_MESSAGE = (
+    "No resource key found, so the tests that call the cloud service "
+    f"cannot run. Set the environment variable '{RESOURCE_KEY_ENV_VAR}' "
+    f"(the older name '{LEGACY_RESOURCE_KEY_ENV_VAR}' is still read). "
+    "Create a resource key for free at "
+    "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=device-detection-python&utm_content=fiftyone_devicedetection_cloud-tests-test_cloud.py&utm_term=resource-key-required")
+
+
+def get_resource_key():
+    """!
+    The resource key from the environment, or None when neither the aligned
+    nor the older variable is set.
+    """
+
+    return (os.environ.get(RESOURCE_KEY_ENV_VAR)
+            or os.environ.get(LEGACY_RESOURCE_KEY_ENV_VAR))
 
 mobile_ua = "Mozilla/5.0 (iPhone; CPU iPhone OS 11_2 like Mac OS X) AppleWebKit/604.4.7 (KHTML, like Gecko) Mobile/15C114"
 
 cloud_request_origin_test_params = [('', True), ('test.com', True), ('51Degrees.com', False)]
 
 class DeviceDetectionTests(unittest.TestCase):
 
+    def setUp(self):
+        self.resource_key = get_resource_key()
+
+        if not self.resource_key:
+            self.skipTest(MISSING_KEY_MESSAGE)
+
+
     def test_pipeline_builder_cloud_engine_init(self):
         """!
         Tests whether the device detection pipeline builder adds the correct engines when initialised with a resource key
         """
 
-        if not "resource_key" in os.environ:
-            return
-
-        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build()
+        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build()
 
         self.assertTrue(pipeline.flow_elements[0].datakey == "cloud")
         self.assertTrue(pipeline.flow_elements[1].datakey == "device")
@@ -56,10 +81,7 @@ def test_properties_cloud(self):
         Tests whether a properties list is created on the cloud engine
         """
 
-        if not "resource_key" in os.environ:
-            return
-
-        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build()
+        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build()
 
         properties = pipeline.flow_elements[1].get_properties()
 
@@ -70,10 +92,7 @@ def test_basic_get_cloud(self):
         Check property lookup works
         """
 
-        if not "resource_key" in os.environ:
-            return
-
-        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build()
+        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build()
 
         fd = pipeline.create_flowdata()
 
@@ -89,7 +108,7 @@ def test_missing_property_service_element_not_found(self):
         not available in any datafile
         """
 
-        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build()
+        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build()
 
         fd = pipeline.create_flowdata()
 
@@ -118,7 +137,7 @@ def test_engine_init_performance(self):
 
         start = time.time()
 
-        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build()
+        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build()
 
         fd = pipeline.create_flowdata()
 
@@ -144,7 +163,7 @@ def test_missing_property_service_not_found_anywhere(self):
         not available in cloud
         """
 
-        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = os.environ['resource_key']).build()
+        pipeline = DeviceDetectionCloudPipelineBuilder(resource_key = self.resource_key).build()
 
         fd = pipeline.create_flowdata()