diff --git a/ci/check-environment-variable-names.Tests.ps1 b/ci/check-environment-variable-names.Tests.ps1 index 7ba15f49..f70f1f79 100644 --- a/ci/check-environment-variable-names.Tests.ps1 +++ b/ci/check-environment-variable-names.Tests.ps1 @@ -31,12 +31,12 @@ Describe 'Find-EnvironmentNameProblem' { @{ Line = 'if "resource_key" in os.environ:' } @{ Line = '$env:resource_key = "key"' } @{ Line = '[Environment]::SetEnvironmentVariable("license_key", $k)' } - @{ Line = 'RESOURCE = env("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY")' } - @{ Line = 'LICENCE = env("_51DEGREES_LICENSE_KEY", "license_key")' } @{ Line = 'LEGACY_RESOURCE_KEY_ENV_VAR = "resource_key"' } - @{ Line = 'legacy_license_key_env_var = "x"' } @{ Line = 'self.baseURL = os.environ.get("FOD_CLOUD_API_URL")' } @{ Line = 'FOD_CLOUD_API_URL = "FOD_CLOUD_API_URL"' } + @{ Line = 'saved = os.environ.pop("FOD_CLOUD_API_URL", None)' } + @{ Line = '$env:FOD_CLOUD_API_URL = "https://cloud.51degrees.com/api/v4/"' } + @{ Line = 'key = env("_51DEGREES_RESOURCE_KEY", "resource_key") or os.environ.get("license_key")' } @{ Line = 'RESOURCE_KEY_ENV_VAR = "resource_key"' } @{ Line = 'ENDPOINT_ENV_VAR = "cloud_endpoint"' } @{ Line = 'ORIGIN_ENV_VAR = "cloud_request_origin"' } @@ -49,7 +49,6 @@ Describe 'Find-EnvironmentNameProblem' { @{ Line = 'env = os.environ.get("IPINTELLIGENCEDATAFILE")' } @{ Line = 'os.environ["ipintelligencetestkey"] = "value"' } @{ Line = 'if "run_performance_tests" in os.environ:' } - @{ Line = 'saved = os.environ.pop("FOD_CLOUD_API_URL", None)' } ) { Get-Kind $Line | Should -Contain 'fallback' } @@ -82,4 +81,85 @@ Describe 'Find-EnvironmentNameProblem' { ) { Get-Kind $Line | Should -BeNullOrEmpty } + + # A variable whose name changed is still read under its old name when + # the new one is not set. That read gives the old name as the argument + # straight after the new one, and it is left alone. + It 'leaves the deprecated read alone' -ForEach @( + @{ Line = 'RESOURCE = env("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY")' } + @{ Line = 'LICENCE = env("_51DEGREES_LICENSE_KEY", "license_key")' } + @{ Line = 'API = env("_51DEGREES_CLOUD_ENDPOINT", "FOD_CLOUD_API_URL")' } + @{ Line = 'read_environment("_51DEGREES_IPI_PATH", "IPINTELLIGENCEDATAFILE")' } + @{ Line = ' "_51DEGREES_RUN_PERFORMANCE_TESTS", "run_performance_tests")' } + @{ Line = "read_environment('_51DEGREES_DD_PATH', '51DEGREES_DD_PATH')" } + ) { + Get-Kind $Line | Should -BeNullOrEmpty + } +} + +Describe 'Find-EnvironmentNameProblem with the old names a file may hold' { + + It 'leaves alone when the file may hold ' -ForEach @( + @{ Line = 'ENDPOINT_ENV_VAR = "cloud_endpoint"'; Allowed = @('cloud_endpoint') } + @{ Line = 'os.environ["resource_key"] = "old-key"'; Allowed = @('resource_key', 'license_key') } + @{ Line = 'DATA_FILE_ENV_VAR = "51DEGREES_DD_PATH"'; Allowed = @('51DEGREES_DD_PATH') } + @{ Line = ' FOD_CLOUD_API_URL'; Allowed = @('FOD_CLOUD_API_URL') } + @{ Line = 'os.environ["RESOURCE_KEY"] = "old-resource-key"'; Allowed = @('RESOURCE_KEY') } + ) { + @(Find-EnvironmentNameProblem $Line $Allowed) | Should -BeNullOrEmpty + } + + It 'finds in when the file may hold only ' -ForEach @( + @{ Line = 'ENDPOINT_ENV_VAR = "cloud_endpoint"'; Allowed = @('resource_key'); Kind = 'fallback' } + @{ Line = 'os.environ["license_key"] = "old-key"'; Allowed = @('resource_key'); Kind = 'fallback' } + @{ Line = 'os.environ["RESOURCE_KEY"] = "old-key"'; Allowed = @('resource_key'); Kind = 'fallback' } + @{ Line = ' IPINTELLIGENCEDATAFILE'; Allowed = @('FOD_CLOUD_API_URL'); Kind = 'fallback' } + @{ Line = 'DATA_FILE_ENV_VAR = "51DEGREES_IPI_PATH"'; Allowed = @('51DEGREES_DD_PATH'); Kind = 'digit' } + @{ Line = ' 51DEGREES_DD_PATH_OTHER'; Allowed = @('51DEGREES_DD_PATH'); Kind = 'digit' } + @{ Line = 'DATA_FILE_ENV_VAR = "51DEGREES_DD_PATH"'; Allowed = @(); Kind = 'digit' } + ) { + @(Find-EnvironmentNameProblem $Line $Allowed | ForEach-Object { $_.Kind }) | + Should -Contain $Kind + } +} + +Describe 'Get-FallbackName' { + + It 'lets hold ' -ForEach @( + @{ Path = 'pipeline/fiftyone_pipeline_did/tox.ini'; Name = 'FOD_CLOUD_API_URL' } + @{ Path = 'device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py'; Name = '51DEGREES_DD_PATH' } + @{ Path = 'device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py'; Name = 'cloud_endpoint' } + @{ Path = 'ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py'; Name = 'IPINTELLIGENCEDATAFILE' } + @{ Path = 'pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py'; Name = 'FOD_CLOUD_API_URL' } + @{ Path = 'pipeline/fiftyone_pipeline_cloudrequestengine/readme.md'; Name = 'FOD_CLOUD_API_URL' } + @{ Path = 'pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py'; Name = 'FOD_CLOUD_API_URL' } + @{ Path = 'pipeline/fiftyone_pipeline_did/tests/test_live_cloud.py'; Name = 'resource_key' } + ) { + @(Get-FallbackName $Path) | Should -Contain $Name + } + + It 'does not let hold ' -ForEach @( + @{ Path = 'pipeline/fiftyone_pipeline_did/tox.ini'; Name = '51DEGREES_DD_PATH' } + @{ Path = 'pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py'; Name = 'resource_key' } + @{ Path = 'pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py'; Name = '51DEGREES_DD_PATH' } + @{ Path = 'ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py'; Name = '51DEGREES_DD_PATH' } + @{ Path = 'device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py'; Name = 'IPINTELLIGENCEDATAFILE' } + ) { + @(Get-FallbackName $Path) | Should -Not -Contain $Name + } + + It 'lets hold no old name' -ForEach @( + @{ Path = 'ci/run-unit-tests.ps1' } + @{ Path = 'readme.md' } + @{ Path = 'pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/pipeline.py' } + @{ Path = 'pipeline/fiftyone_pipeline_core/readme.md' } + @{ Path = 'device-detection/fiftyone_devicedetection_examples/tests/test_cloudexamples.py' } + @{ Path = 'device-detection/ci/run-integration-tests.ps1' } + @{ Path = 'pipeline/fiftyone_pipeline_did/tests/test_did_client_live.py' } + @{ Path = 'location/examples/cloud/gettingstarted.py' } + @{ Path = 'some/new/package/tox.ini' } + @{ Path = 'some/new/example_utils.py' } + ) { + @(Get-FallbackName $Path) | Should -BeNullOrEmpty + } } diff --git a/ci/check-environment-variable-names.ps1 b/ci/check-environment-variable-names.ps1 index 6c9766d2..5283cd81 100644 --- a/ci/check-environment-variable-names.ps1 +++ b/ci/check-environment-variable-names.ps1 @@ -1,8 +1,15 @@ # Every 51Degrees environment variable name in this repository starts with an # underscore, because a POSIX shell cannot set a name that starts with a digit, -# and each variable has one name with nothing read in its place. This reads -# every file git tracks, which leaves out build output and the submodules, and -# fails on any name that starts with the digit and on any fallback name. +# and everything documents that one name. This reads every file git tracks, +# which leaves out build output and the submodules, and fails on any name +# that starts with the digit and on any old name. +# +# A variable whose name changed is still read under its old name when the new +# one is not set, which is deprecated. So an old name is left alone in the two +# places that read needs it. One is a line where the old name is the argument +# straight after its _51DEGREES_ name, which is how each deprecated read is +# written. The other is the files $FallbackNames lists, each of which may hold +# the old names it is listed with and no others. # # Names are matched in any case, because Windows reads an environment # variable name in any case, so a lower case old name is still the old name @@ -31,9 +38,8 @@ $script:DigitInitialPattern = [regex]::new( "(?") CREATE_KEY_URL = "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=live_cloud.py&utm_term=resource-key-required" +def _report_old_name(name, old_name): + """Prints, once for each old name, that it is deprecated and which + name replaces it.""" + if old_name in _reported_old_names: + return + _reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated.""" + value = os.environ.get(name, "").strip() + if value: + return value + value = os.environ.get(old_name, "").strip() + if not value: + return None + _report_old_name(name, old_name) + return value + + +def is_set(name, old_name): + """Whether the environment variable name is set, whatever it holds, + which is how a switch is read. When it is not set, the name the + variable had before counts, and using it prints once that the old name + is deprecated.""" + if name in os.environ: + return True + if old_name not in os.environ: + return False + _report_old_name(name, old_name) + return True + + def find_resource_key(names=RESOURCE_KEY_NAMES): - """The first resource key set under any of the names, or None.""" + """The first resource key set under any of the names, or None. When + none of RESOURCE_KEY_NAMES is set, the deprecated old name is read.""" for name in names: value = os.environ.get(name, "").strip() if value: return value + if names is RESOURCE_KEY_NAMES: + return read_environment(names[0], OLD_RESOURCE_KEY_NAME) return None diff --git a/device-detection/fiftyone_devicedetection_cloud/tox.ini b/device-detection/fiftyone_devicedetection_cloud/tox.ini index 27672a13..28643c83 100644 --- a/device-detection/fiftyone_devicedetection_cloud/tox.ini +++ b/device-detection/fiftyone_devicedetection_cloud/tox.ini @@ -25,8 +25,10 @@ commands = # which leaves the live tests out of the unit step. pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_RESOURCE_KEY_BESPOKE _51DEGREES_LICENSE_KEY + license_key PYTEST_ADDOPTS [testenv:pre-publish] diff --git a/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py b/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py index 873f6568..98f38b8d 100644 --- a/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py +++ b/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/example_utils.py @@ -26,6 +26,7 @@ import shlex import shutil import subprocess +import sys from pathlib import Path @@ -42,12 +43,35 @@ class ExampleUtils: # resource key variable starts with "_51DEGREES_RESOURCE_KEY". RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY" - ENDPOINT_ENV_VAR = "_51DEGREES_CLOUD_ENDPOINT" + # Deprecated. Use RESOURCE_KEY_ENV_VAR. The name the resource key + # variable had before, read when RESOURCE_KEY_ENV_VAR is not set. + LEGACY_RESOURCE_KEY_ENV_VAR = "resource_key" + + # The environment variable the data file update example reads a licence + # key from. + LICENSE_KEY_ENV_VAR = "_51DEGREES_LICENSE_KEY" + + # Deprecated. Use LICENSE_KEY_ENV_VAR. The name the licence key + # variable had before, read when LICENSE_KEY_ENV_VAR is not set. + LEGACY_LICENSE_KEY_ENV_VAR = "license_key" + + # The environment variable naming a cloud service other than the public + # one. + CLOUD_ENDPOINT_ENV_VAR = "_51DEGREES_CLOUD_ENDPOINT" + + # Deprecated. Use CLOUD_ENDPOINT_ENV_VAR. The name that variable had + # before, read when CLOUD_ENDPOINT_ENV_VAR is not set. + ENDPOINT_ENV_VAR = "cloud_endpoint" # The environment variable an on-premise example reads the path of its # device detection data file from, when one is given, in place of the # file named in its configuration. - DATA_FILE_ENV_VAR = "_51DEGREES_DD_PATH" + DD_PATH_ENV_VAR = "_51DEGREES_DD_PATH" + + # Deprecated. Use DD_PATH_ENV_VAR. The name that variable had before, + # read when DD_PATH_ENV_VAR is not set. A POSIX shell cannot set it, + # because it starts with a digit. + DATA_FILE_ENV_VAR = "51DEGREES_DD_PATH" # The data repository's device detection folder, from the root of a # checkout of this repository. Each file the data repository links to @@ -161,7 +185,24 @@ def get_no_profiles_message(): @staticmethod def get_resource_key(): - return ExampleUtils.__get_env_variable(ExampleUtils.RESOURCE_KEY_ENV_VAR) + return ExampleUtils.__get_env_variable( + ExampleUtils.RESOURCE_KEY_ENV_VAR, + ExampleUtils.LEGACY_RESOURCE_KEY_ENV_VAR) + + @staticmethod + def get_license_key(): + """! + The licence key from the environment, or an empty string. The old + name is also read in upper case, as it was before. + """ + + return ( + ExampleUtils.__get_env_variable( + ExampleUtils.LICENSE_KEY_ENV_VAR, + ExampleUtils.LEGACY_LICENSE_KEY_ENV_VAR) + or ExampleUtils.__get_env_variable( + ExampleUtils.LICENSE_KEY_ENV_VAR, + ExampleUtils.LEGACY_LICENSE_KEY_ENV_VAR.upper())) @staticmethod def get_missing_resource_key_message(): @@ -176,18 +217,41 @@ def get_missing_resource_key_message(): @staticmethod def get_data_file_path(): - return ExampleUtils.__get_env_variable(ExampleUtils.DATA_FILE_ENV_VAR) + return ExampleUtils.__get_env_variable( + ExampleUtils.DD_PATH_ENV_VAR, ExampleUtils.DATA_FILE_ENV_VAR) @staticmethod def get_cloud_endpoint(): - return ExampleUtils.__get_env_variable(ExampleUtils.ENDPOINT_ENV_VAR) + return ExampleUtils.__get_env_variable( + ExampleUtils.CLOUD_ENDPOINT_ENV_VAR, + ExampleUtils.ENDPOINT_ENV_VAR) + + # The old names already reported as deprecated, so each is reported + # once. + __reported_old_names = set() @staticmethod - def __get_env_variable(name): - if name in os.environ: - return os.environ[name] - else: + def __get_env_variable(name, old_name): + """! + The value of the environment variable name, or an empty string. + When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated. + """ + + value = os.environ.get(name) + if value: + return value + value = os.environ.get(old_name) + if not value: return "" + if old_name not in ExampleUtils.__reported_old_names: + ExampleUtils.__reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + return value # Find the specified file. A path to a file that exists is returned as # it is. Next, the data repository's folder is checked in the working diff --git a/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/onpremise/datafileupdate_console.py b/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/onpremise/datafileupdate_console.py index eeaa8492..109f9fc3 100644 --- a/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/onpremise/datafileupdate_console.py +++ b/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/onpremise/datafileupdate_console.py @@ -171,7 +171,12 @@ from fiftyone_pipeline_engines.datafile_update_service import DataFileUpdateService from fiftyone_pipeline_engines.datafile_update_service import UpdateStatus -UPDATE_EXAMPLE_LICENSE_KEY_NAME = "_51DEGREES_LICENSE_KEY" +# The environment variable checked for the licence key when none is given +# on the command line. +UPDATE_EXAMPLE_LICENSE_KEY_VARIABLE = ExampleUtils.LICENSE_KEY_ENV_VAR +# Deprecated. Use UPDATE_EXAMPLE_LICENSE_KEY_VARIABLE. The name that +# variable had before, read when the new one is not set. +UPDATE_EXAMPLE_LICENSE_KEY_NAME = ExampleUtils.LEGACY_LICENSE_KEY_ENV_VAR # Where to ask for a licence key and a copy of the Enterprise data file, tagged with this # example. CONTACT_US = "https://51degrees.com/contact-us?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=fiftyone_devicedetection_examples-src-fiftyone_devicedetection_examples-onpremise-datafileupdate_console.py&utm_term=licence-key" @@ -215,7 +220,7 @@ def explain_license_key(self, data_file, output): output("Updating the data file downloads the Enterprise data file, " "which needs a licence key. Give the key as the first command line " f"argument, or in the environment variable " - f"'{UPDATE_EXAMPLE_LICENSE_KEY_NAME}'. Contact us for a key and a " + f"'{UPDATE_EXAMPLE_LICENSE_KEY_VARIABLE}'. Contact us for a key and a " f"copy at {CONTACT_US}") def run(self, data_file, license_key, interactive, logger, output): @@ -223,7 +228,7 @@ def run(self, data_file, license_key, interactive, logger, output): # try to find a license key if not license_key: - license_key = KeyUtils.get_named_key(UPDATE_EXAMPLE_LICENSE_KEY_NAME) + license_key = ExampleUtils.get_license_key() if not license_key: self.explain_license_key(data_file, output) diff --git a/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/onpremise/gettingstarted_web/app.py b/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/onpremise/gettingstarted_web/app.py index 080b512f..b3b9dcfc 100644 --- a/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/onpremise/gettingstarted_web/app.py +++ b/device-detection/fiftyone_devicedetection_examples/src/fiftyone_devicedetection_examples/onpremise/gettingstarted_web/app.py @@ -199,7 +199,7 @@ def build_config(): if not os.path.exists(envDataFile): raise Exception("The device detection data file " + f"'{envDataFile}' named in the environment variable " + - f"'{ExampleUtils.DATA_FILE_ENV_VAR}' does not exist.") + f"'{ExampleUtils.DD_PATH_ENV_VAR}' does not exist.") ExampleUtils.set_data_file_in_config(config, envDataFile) return config @@ -226,7 +226,7 @@ def build_config(): "`git submodule update --init --recursive`, and run this " + "example again to download the file. Otherwise, ensure that the filename " + "is correct in config.json, or set the environment variable " + - f"'{ExampleUtils.DATA_FILE_ENV_VAR}' to the path of the data file.") + f"'{ExampleUtils.DD_PATH_ENV_VAR}' to the path of the data file.") return config diff --git a/device-detection/fiftyone_devicedetection_examples/tests/live_cloud.py b/device-detection/fiftyone_devicedetection_examples/tests/live_cloud.py index 23eb9f4f..36bc26ad 100644 --- a/device-detection/fiftyone_devicedetection_examples/tests/live_cloud.py +++ b/device-detection/fiftyone_devicedetection_examples/tests/live_cloud.py @@ -22,14 +22,16 @@ """Where the tests that call the live 51Degrees cloud service find their resource key, and what they fail with when there is none or when the -answer lacks what they asked for. +answer lacks what they asked for. Also how a test reads any setting from +an environment variable whose name has changed. -The same file is in each test folder whose tests call the live cloud -service. There is no fallback to a skip, because a cloud test that passes -without calling the cloud has proved nothing. +The same file is in each test folder whose tests read a setting from the +environment. There is no fallback to a skip, because a cloud test that +passes without calling the cloud has proved nothing. """ import os +import sys # Tried in this order. _51DEGREES_RESOURCE_KEY is first because the Cloud # 51Did workflow sets it to each key in turn, and each product's @@ -39,18 +41,68 @@ "_51DEGREES_RESOURCE_KEY_BESPOKE", ) +# Deprecated. The name the resource key variable had before, read when +# none of RESOURCE_KEY_NAMES is set. +OLD_RESOURCE_KEY_NAME = "resource_key" + +# The old names already reported as deprecated, so each is reported once. +_reported_old_names = set() + ACCESSIBLE_PROPERTIES_URL = ( "https://cloud.51degrees.com/api/v4/accessibleproperties?resource=") CREATE_KEY_URL = "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=live_cloud.py&utm_term=resource-key-required" +def _report_old_name(name, old_name): + """Prints, once for each old name, that it is deprecated and which + name replaces it.""" + if old_name in _reported_old_names: + return + _reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated.""" + value = os.environ.get(name, "").strip() + if value: + return value + value = os.environ.get(old_name, "").strip() + if not value: + return None + _report_old_name(name, old_name) + return value + + +def is_set(name, old_name): + """Whether the environment variable name is set, whatever it holds, + which is how a switch is read. When it is not set, the name the + variable had before counts, and using it prints once that the old name + is deprecated.""" + if name in os.environ: + return True + if old_name not in os.environ: + return False + _report_old_name(name, old_name) + return True + + def find_resource_key(names=RESOURCE_KEY_NAMES): - """The first resource key set under any of the names, or None.""" + """The first resource key set under any of the names, or None. When + none of RESOURCE_KEY_NAMES is set, the deprecated old name is read.""" for name in names: value = os.environ.get(name, "").strip() if value: return value + if names is RESOURCE_KEY_NAMES: + return read_environment(names[0], OLD_RESOURCE_KEY_NAME) return None diff --git a/device-detection/fiftyone_devicedetection_examples/tests/test_environment_names.py b/device-detection/fiftyone_devicedetection_examples/tests/test_environment_names.py new file mode 100644 index 00000000..be74700c --- /dev/null +++ b/device-detection/fiftyone_devicedetection_examples/tests/test_environment_names.py @@ -0,0 +1,162 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""Checks that the example helper still reads each setting under the name +its environment variable had before, when the new name is not set, and +prints once that the old name is deprecated. These need no resource key. +""" + +import contextlib +import io +import os +import unittest +from unittest import mock + +from fiftyone_devicedetection_examples.example_utils import ExampleUtils +from fiftyone_devicedetection_shared.key_utils import KeyUtils + +# Each reader, with the new name of its variable and the old one. +PAIRS = { + "get_resource_key": ("_51DEGREES_RESOURCE_KEY", "resource_key"), + "get_license_key": ("_51DEGREES_LICENSE_KEY", "license_key"), + "get_cloud_endpoint": ("_51DEGREES_CLOUD_ENDPOINT", "cloud_endpoint"), + "get_data_file_path": ("_51DEGREES_DD_PATH", "51DEGREES_DD_PATH"), +} + +# Each constant that names a variable, with the value it has to hold. The +# ones that held an old name hold it still. +CONSTANTS = { + "RESOURCE_KEY_ENV_VAR": "_51DEGREES_RESOURCE_KEY", + "LEGACY_RESOURCE_KEY_ENV_VAR": "resource_key", + "LICENSE_KEY_ENV_VAR": "_51DEGREES_LICENSE_KEY", + "LEGACY_LICENSE_KEY_ENV_VAR": "license_key", + "CLOUD_ENDPOINT_ENV_VAR": "_51DEGREES_CLOUD_ENDPOINT", + "ENDPOINT_ENV_VAR": "cloud_endpoint", + "DD_PATH_ENV_VAR": "_51DEGREES_DD_PATH", + "DATA_FILE_ENV_VAR": "51DEGREES_DD_PATH", +} + + +class OldNameTests(unittest.TestCase): + + def setUp(self): + # Each test starts with none of the names set and no old name + # reported yet, and what it sets is put back afterwards. + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + for pair in PAIRS.values(): + for name in pair: + os.environ.pop(name, None) + os.environ.pop(name.upper(), None) + reported = mock.patch.object( + ExampleUtils, "_ExampleUtils__reported_old_names", set()) + reported.start() + self.addCleanup(reported.stop) + + def printed(self, call): + """What a call returned and what it printed to the error stream.""" + errors = io.StringIO() + with contextlib.redirect_stderr(errors): + value = call() + return value, errors.getvalue() + + def test_each_setting_is_read_from_its_old_name(self): + for getter, (name, old_name) in PAIRS.items(): + with self.subTest(getter=getter): + os.environ[old_name] = "old-value" + value, errors = self.printed(getattr(ExampleUtils, getter)) + self.assertEqual("old-value", value) + self.assertIn( + old_name + " environment variable is deprecated", errors) + self.assertIn(name, errors) + del os.environ[old_name] + + def test_new_name_wins_and_nothing_is_printed(self): + for getter, (name, old_name) in PAIRS.items(): + with self.subTest(getter=getter): + os.environ[name] = "new-value" + os.environ[old_name] = "old-value" + value, errors = self.printed(getattr(ExampleUtils, getter)) + self.assertEqual("new-value", value) + self.assertEqual("", errors) + del os.environ[name] + del os.environ[old_name] + + def test_neither_name_gives_nothing(self): + for getter in PAIRS: + with self.subTest(getter=getter): + value, errors = self.printed(getattr(ExampleUtils, getter)) + self.assertFalse(value) + self.assertEqual("", errors) + + def test_old_name_is_reported_once(self): + getter, (name, old_name) = next(iter(PAIRS.items())) + os.environ[old_name] = "old-value" + + def twice(): + return (getattr(ExampleUtils, getter)(), + getattr(ExampleUtils, getter)()) + + value, errors = self.printed(twice) + self.assertEqual(("old-value", "old-value"), value) + self.assertEqual( + 1, errors.count(old_name + " environment variable is deprecated")) + + def test_constants_hold_the_old_and_the_new_names(self): + for constant, value in CONSTANTS.items(): + with self.subTest(constant=constant): + self.assertEqual(value, getattr(ExampleUtils, constant)) + + def test_licence_key_is_read_from_the_old_name_in_upper_case(self): + os.environ.pop("LICENSE_KEY", None) + os.environ["LICENSE_KEY"] = "old-value" + value, _ = self.printed(ExampleUtils.get_license_key) + self.assertEqual("old-value", value) + + +class KeyUtilsTests(unittest.TestCase): + + LOWER = "_51degrees_test_named_key" + UPPER = "_51DEGREES_TEST_NAMED_KEY" + + def setUp(self): + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + os.environ.pop(self.LOWER, None) + os.environ.pop(self.UPPER, None) + + def test_key_is_read_from_the_name_given(self): + os.environ[self.LOWER] = "value" + self.assertEqual("value", KeyUtils.get_named_key(self.LOWER)) + + def test_key_is_read_from_the_name_in_upper_case(self): + os.environ[self.UPPER] = "value" + self.assertEqual("value", KeyUtils.get_named_key(self.LOWER)) + + def test_missing_key_gives_none(self): + self.assertIsNone(KeyUtils.get_named_key(self.LOWER)) + + +if __name__ == "__main__": + unittest.main() diff --git a/device-detection/fiftyone_devicedetection_examples/tests/test_examples_without_settings.py b/device-detection/fiftyone_devicedetection_examples/tests/test_examples_without_settings.py index 555c0443..6e7fbef4 100644 --- a/device-detection/fiftyone_devicedetection_examples/tests/test_examples_without_settings.py +++ b/device-detection/fiftyone_devicedetection_examples/tests/test_examples_without_settings.py @@ -45,7 +45,7 @@ from fiftyone_devicedetection_shared.example_constants import LITE_DATAFILE_NAME # The settings the examples read, which a fresh clone does not have. -SETTINGS = ["_51DEGREES_LICENSE_KEY"] +SETTINGS = ["_51DEGREES_LICENSE_KEY", "license_key", "LICENSE_KEY"] # The evidence records the offline processing test reads. The example's own # file has 20,000, which take minutes. diff --git a/device-detection/fiftyone_devicedetection_examples/tests/test_onpremiseexamples.py b/device-detection/fiftyone_devicedetection_examples/tests/test_onpremiseexamples.py index aa200bba..7e56aefd 100644 --- a/device-detection/fiftyone_devicedetection_examples/tests/test_onpremiseexamples.py +++ b/device-detection/fiftyone_devicedetection_examples/tests/test_onpremiseexamples.py @@ -22,7 +22,6 @@ import os import unittest -from fiftyone_devicedetection_shared.key_utils import KeyUtils from fiftyone_pipeline_core.logger import Logger from fiftyone_devicedetection_examples.example_utils import ExampleUtils from fiftyone_devicedetection_examples.onpremise.gettingstarted_console import GettingStartedConsole @@ -30,7 +29,12 @@ from fiftyone_devicedetection_examples.onpremise.offlineprocessing import OfflineProcessing from fiftyone_devicedetection_examples.onpremise.match_metrics import MatchMetricsConsole from fiftyone_devicedetection_examples.onpremise.datafileupdate_console import DataFileUpdateConsole -from fiftyone_devicedetection_examples.onpremise.datafileupdate_console import UPDATE_EXAMPLE_LICENSE_KEY_NAME + +from .live_cloud import is_set + +# Deprecated old name, read when the new one is not set. +RUN_PERFORMANCE_TESTS = ( + "_51DEGREES_RUN_PERFORMANCE_TESTS", "run_performance_tests") class DeviceDetectionExampleTests(unittest.TestCase): @@ -59,12 +63,12 @@ def test_onpremise_match_metrics(self): def test_onpremise_datafileupdate_console(self): example = DataFileUpdateConsole() - license_key = KeyUtils.get_named_key(UPDATE_EXAMPLE_LICENSE_KEY_NAME) + license_key = ExampleUtils.get_license_key() example.run("Enterprise-HashV41.hash", license_key, False, self.logger, print) def test_onpremise_offline_processing(self): # Only run if environment variable set - if "_51DEGREES_RUN_PERFORMANCE_TESTS" in os.environ: + if is_set(*RUN_PERFORMANCE_TESTS): example = OfflineProcessing() with open(self.evidence_file, "r") as input: with open("./offlineprocessing-output.yml", "w") as output: @@ -88,5 +92,5 @@ def test_onpremise_offline_processing_with_tabs(self): def test_onpremise_performance(self): # Only run if environment variable set - if "_51DEGREES_RUN_PERFORMANCE_TESTS" in os.environ: + if is_set(*RUN_PERFORMANCE_TESTS): import fiftyone_devicedetection_examples.onpremise.performance diff --git a/device-detection/fiftyone_devicedetection_examples/tests/test_onpremisegettingstartedweb.py b/device-detection/fiftyone_devicedetection_examples/tests/test_onpremisegettingstartedweb.py index a997f5c7..5c8978ee 100644 --- a/device-detection/fiftyone_devicedetection_examples/tests/test_onpremisegettingstartedweb.py +++ b/device-detection/fiftyone_devicedetection_examples/tests/test_onpremisegettingstartedweb.py @@ -81,7 +81,7 @@ def test_data_file_from_environment(self): with tempfile.NamedTemporaryFile(suffix=".hash", delete=False) as file: path = file.name try: - with mock.patch.dict(os.environ, {ExampleUtils.DATA_FILE_ENV_VAR: path}): + with mock.patch.dict(os.environ, {ExampleUtils.DD_PATH_ENV_VAR: path}): config = GettingStartedWeb.build_config() self.assertEqual( os.path.abspath(path), @@ -93,7 +93,7 @@ def test_data_file_from_environment(self): # name, rather than the example silently falling back to config.json. def test_missing_data_file_from_environment(self): missing = os.path.join(tempfile.gettempdir(), "no-such-51degrees-file.hash") - with mock.patch.dict(os.environ, {ExampleUtils.DATA_FILE_ENV_VAR: missing}): + with mock.patch.dict(os.environ, {ExampleUtils.DD_PATH_ENV_VAR: missing}): with self.assertRaises(Exception) as context: GettingStartedWeb.build_config() - self.assertIn(ExampleUtils.DATA_FILE_ENV_VAR, str(context.exception)) + self.assertIn(ExampleUtils.DD_PATH_ENV_VAR, str(context.exception)) diff --git a/device-detection/fiftyone_devicedetection_examples/tox.ini b/device-detection/fiftyone_devicedetection_examples/tox.ini index ca26fa15..f7d9f947 100644 --- a/device-detection/fiftyone_devicedetection_examples/tox.ini +++ b/device-detection/fiftyone_devicedetection_examples/tox.ini @@ -33,9 +33,12 @@ commands = {[common]commands} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_RESOURCE_KEY_BESPOKE _51DEGREES_LICENSE_KEY + license_key _51DEGREES_RUN_PERFORMANCE_TESTS + run_performance_tests [testenv:pre-publish] deps = diff --git a/device-detection/fiftyone_devicedetection_onpremise/tox.ini b/device-detection/fiftyone_devicedetection_onpremise/tox.ini index 261fd422..4e3d7d7c 100644 --- a/device-detection/fiftyone_devicedetection_onpremise/tox.ini +++ b/device-detection/fiftyone_devicedetection_onpremise/tox.ini @@ -33,7 +33,9 @@ commands = {[common]commands} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key TMPDIR TEMP TMP diff --git a/device-detection/fiftyone_devicedetection_shared/src/fiftyone_devicedetection_shared/key_utils.py b/device-detection/fiftyone_devicedetection_shared/src/fiftyone_devicedetection_shared/key_utils.py index e4ea13a4..a6ef6753 100644 --- a/device-detection/fiftyone_devicedetection_shared/src/fiftyone_devicedetection_shared/key_utils.py +++ b/device-detection/fiftyone_devicedetection_shared/src/fiftyone_devicedetection_shared/key_utils.py @@ -21,10 +21,31 @@ # ********************************************************************* import os +import warnings class KeyUtils: - # A key from the environment variable of exactly that name. + # The upper case names already reported as deprecated, so each is + # reported once. + _reported_upper_case = set() + + # A key from the environment variable of that name. When that variable + # is unset or empty, the variable with the same name in upper case is + # read, which is deprecated and reported once with a FutureWarning. @staticmethod def get_named_key(key_name): - return os.environ.get(key_name) + value = os.environ.get(key_name) + upper = key_name.upper() + if value or upper == key_name: + return value + upper_value = os.environ.get(upper) + if not upper_value: + return value or upper_value + if upper not in KeyUtils._reported_upper_case: + KeyUtils._reported_upper_case.add(upper) + warnings.warn( + "Reading a key from the upper case form of its variable name " + "is deprecated. The key was read from {0} because {1} is not " + "set, so set {1} to the same value.".format(upper, key_name), + FutureWarning, stacklevel=2) + return upper_value diff --git a/ip-intelligence/fiftyone_ipintelligence_cloud/tox.ini b/ip-intelligence/fiftyone_ipintelligence_cloud/tox.ini index 996a428f..f4a02df3 100644 --- a/ip-intelligence/fiftyone_ipintelligence_cloud/tox.ini +++ b/ip-intelligence/fiftyone_ipintelligence_cloud/tox.ini @@ -23,7 +23,9 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_ipintelligence_cloud {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key [testenv:pre-publish] package = skip diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/getallproperties_console.py b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/getallproperties_console.py index 75c8c9d9..fc950959 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/getallproperties_console.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/getallproperties_console.py @@ -102,7 +102,7 @@ def main(argv): else: logger.log("error", "No resource key specified in the environment variable " - f"'{ExampleUtils.RESOURCE_KEY_ENV_VAR}'. The 51Degrees cloud " + f"'{ExampleUtils.CLOUD_RESOURCE_KEY_ENV_VAR}'. The 51Degrees cloud " "service is accessed using a 'ResourceKey'. For more information " "see " "https://51degrees.com/documentation/_services__cloud__resource_keys.html?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=fiftyone_ipintelligence_examples-src-fiftyone_ipintelligence_examples-cloud-getallproperties_console.py&utm_term=resource-key-required. " diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/gettingstarted_console.py b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/gettingstarted_console.py index 7118db82..48bfe83b 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/gettingstarted_console.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/gettingstarted_console.py @@ -169,7 +169,7 @@ def main(argv): else: logger.log("error", "No resource key specified in the environment variable " - f"'{ExampleUtils.RESOURCE_KEY_ENV_VAR}'. The 51Degrees cloud " + f"'{ExampleUtils.CLOUD_RESOURCE_KEY_ENV_VAR}'. The 51Degrees cloud " "service is accessed using a 'ResourceKey'. For more " "information see " "https://51degrees.com/documentation/_services__cloud__resource_keys.html?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=fiftyone_ipintelligence_examples-src-fiftyone_ipintelligence_examples-cloud-gettingstarted_console.py&utm_term=resource-key-required. " diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/gettingstarted_web/app.py b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/gettingstarted_web/app.py index 04b2fcf0..59183683 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/gettingstarted_web/app.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/gettingstarted_web/app.py @@ -91,7 +91,7 @@ def main(resource_key = None): if not resource_key: raise Exception( "No resource key specified in the environment variable " - f"'{ExampleUtils.RESOURCE_KEY_ENV_VAR}'. Obtain one from " + f"'{ExampleUtils.CLOUD_RESOURCE_KEY_ENV_VAR}'. Obtain one from " "https://configure.51degrees.com/?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=fiftyone_ipintelligence_examples-src-fiftyone_ipintelligence_examples-cloud-gettingstarted_web-app.py&utm_term=resource-key-required") build_pipeline(resource_key) app.run(debug=False) diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/metadata_console.py b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/metadata_console.py index 60db3582..3d6d8f00 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/metadata_console.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/metadata_console.py @@ -101,7 +101,7 @@ def main(argv): else: logger.log("error", "No resource key specified in the environment variable " - f"'{ExampleUtils.RESOURCE_KEY_ENV_VAR}'. The 51Degrees cloud " + f"'{ExampleUtils.CLOUD_RESOURCE_KEY_ENV_VAR}'. The 51Degrees cloud " "service is accessed using a 'ResourceKey'. For more information " "see " "https://51degrees.com/documentation/_services__cloud__resource_keys.html?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=fiftyone_ipintelligence_examples-src-fiftyone_ipintelligence_examples-cloud-metadata_console.py&utm_term=resource-key-required. " diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/mixed_console.py b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/mixed_console.py index d4559a10..2c228035 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/mixed_console.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/cloud/mixed_console.py @@ -169,7 +169,7 @@ def main(argv): else: logger.log("error", "No resource key specified in the environment variable " - f"'{ExampleUtils.RESOURCE_KEY_ENV_VAR}'. The 51Degrees cloud " + f"'{ExampleUtils.CLOUD_RESOURCE_KEY_ENV_VAR}'. The 51Degrees cloud " "service is accessed using a 'ResourceKey'. A resource key " "configured with both device detection and IP intelligence " "properties can be created at https://configure.51degrees.com/?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=fiftyone_ipintelligence_examples-src-fiftyone_ipintelligence_examples-cloud-mixed_console.py&utm_term=resource-key-required.") diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py index b973e2a4..bc261253 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/example_utils.py @@ -25,6 +25,7 @@ import shlex import shutil import subprocess +import sys from pathlib import Path @@ -38,13 +39,35 @@ class ExampleUtils: # The default environment variable used to get the resource key # to use when running examples. - RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY" + CLOUD_RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY" - ENDPOINT_ENV_VAR = "_51DEGREES_CLOUD_ENDPOINT" + # Deprecated. Use CLOUD_RESOURCE_KEY_ENV_VAR. The name the resource key + # variable had before, read when CLOUD_RESOURCE_KEY_ENV_VAR is not set. + RESOURCE_KEY_ENV_VAR = "resource_key" + + # The environment variable the data file update example reads a licence + # key from. + LICENSE_KEY_ENV_VAR = "_51DEGREES_LICENSE_KEY" + + # Deprecated. Use LICENSE_KEY_ENV_VAR. The name the licence key + # variable had before, read when LICENSE_KEY_ENV_VAR is not set. + LEGACY_LICENSE_KEY_ENV_VAR = "license_key" + + # The environment variable naming a cloud service other than the public + # one. + CLOUD_ENDPOINT_ENV_VAR = "_51DEGREES_CLOUD_ENDPOINT" + + # Deprecated. Use CLOUD_ENDPOINT_ENV_VAR. The name that variable had + # before, read when CLOUD_ENDPOINT_ENV_VAR is not set. + ENDPOINT_ENV_VAR = "cloud_endpoint" # The default environment variable used to get the data file path # to use when running examples. - DATA_FILE_ENV_VAR = "_51DEGREES_IPI_PATH" + IPI_PATH_ENV_VAR = "_51DEGREES_IPI_PATH" + + # Deprecated. Use IPI_PATH_ENV_VAR. The name that variable had before, + # read when IPI_PATH_ENV_VAR is not set. + DATA_FILE_ENV_VAR = "IPINTELLIGENCEDATAFILE" # The data repository's IP intelligence folder, from the root of a # checkout of this repository. Each file the data repository links to @@ -123,11 +146,39 @@ def get_value(ip_data, property): @staticmethod def get_resource_key(): - return ExampleUtils.__get_env_variable(ExampleUtils.RESOURCE_KEY_ENV_VAR) + """! + The resource key from the environment, or None. The old name is + also read in upper case, as it was before. + """ + + return ( + ExampleUtils.__get_env_variable( + ExampleUtils.CLOUD_RESOURCE_KEY_ENV_VAR, + ExampleUtils.RESOURCE_KEY_ENV_VAR) + or ExampleUtils.__get_env_variable( + ExampleUtils.CLOUD_RESOURCE_KEY_ENV_VAR, + ExampleUtils.RESOURCE_KEY_ENV_VAR.upper())) + + @staticmethod + def get_license_key(): + return ExampleUtils.__get_env_variable( + ExampleUtils.LICENSE_KEY_ENV_VAR, + ExampleUtils.LEGACY_LICENSE_KEY_ENV_VAR) @staticmethod def get_cloud_endpoint(): - return ExampleUtils.__get_env_variable(ExampleUtils.ENDPOINT_ENV_VAR) + """! + The cloud endpoint from the environment, or None. The old name is + also read in upper case, as it was before. + """ + + return ( + ExampleUtils.__get_env_variable( + ExampleUtils.CLOUD_ENDPOINT_ENV_VAR, + ExampleUtils.ENDPOINT_ENV_VAR) + or ExampleUtils.__get_env_variable( + ExampleUtils.CLOUD_ENDPOINT_ENV_VAR, + ExampleUtils.ENDPOINT_ENV_VAR.upper())) @staticmethod def find_file(file_name): @@ -140,7 +191,8 @@ def find_file(file_name): when it is missing. """ - env = os.environ.get(ExampleUtils.DATA_FILE_ENV_VAR) + env = ExampleUtils.__get_env_variable( + ExampleUtils.IPI_PATH_ENV_VAR, ExampleUtils.DATA_FILE_ENV_VAR) if env and os.path.basename(env) == file_name and os.path.isfile(env): return env @@ -376,6 +428,29 @@ def check_data_file(pipeline, logger): f"or cxx/data/get-data-files.ps1 ip-intelligence -Update " f"on Windows.") + # The old names already reported as deprecated, so each is reported + # once. + __reported_old_names = set() + @staticmethod - def __get_env_variable(name): - return os.environ.get(name) + def __get_env_variable(name, old_name): + """! + The value of the environment variable name, or None. When name is + unset or empty the name the variable had before is read, and using + it prints once that the old name is deprecated. + """ + + value = os.environ.get(name) + if value: + return value + value = os.environ.get(old_name) + if not value: + return None + if old_name not in ExampleUtils.__reported_old_names: + ExampleUtils.__reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + return value diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/onpremise/datafileupdate_console.py b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/onpremise/datafileupdate_console.py index 1ef6fcf4..809a93ce 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/onpremise/datafileupdate_console.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/src/fiftyone_ipintelligence_examples/onpremise/datafileupdate_console.py @@ -76,7 +76,11 @@ # Environment variable checked for the license key if one is not supplied as # a command line argument. -UPDATE_EXAMPLE_LICENSE_KEY_NAME = "_51DEGREES_LICENSE_KEY" +UPDATE_EXAMPLE_LICENSE_KEY_VARIABLE = ExampleUtils.LICENSE_KEY_ENV_VAR + +# Deprecated. Use UPDATE_EXAMPLE_LICENSE_KEY_VARIABLE. The name that +# variable had before, read when the new one is not set. +UPDATE_EXAMPLE_LICENSE_KEY_NAME = ExampleUtils.LEGACY_LICENSE_KEY_ENV_VAR # Where to ask for a licence key and a copy of the Enterprise data file, tagged with this # example. @@ -130,7 +134,7 @@ def explain_license_key(self, data_file, output): output("Updating the data file downloads the Enterprise data file, " "which needs a licence key. Give the key as the first command " "line argument, or in the environment variable " - f"'{UPDATE_EXAMPLE_LICENSE_KEY_NAME}'. Contact us for a key " + f"'{UPDATE_EXAMPLE_LICENSE_KEY_VARIABLE}'. Contact us for a key " f"and a copy at {CONTACT_US}") def run(self, data_file, license_key, interactive, logger, output): @@ -138,7 +142,7 @@ def run(self, data_file, license_key, interactive, logger, output): # Try to find a license key in the environment if not supplied. if not license_key: - license_key = os.environ.get(UPDATE_EXAMPLE_LICENSE_KEY_NAME) + license_key = ExampleUtils.get_license_key() if not license_key: self.explain_license_key(data_file, output) diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/tests/live_cloud.py b/ip-intelligence/fiftyone_ipintelligence_examples/tests/live_cloud.py index 23eb9f4f..36bc26ad 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/tests/live_cloud.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/tests/live_cloud.py @@ -22,14 +22,16 @@ """Where the tests that call the live 51Degrees cloud service find their resource key, and what they fail with when there is none or when the -answer lacks what they asked for. +answer lacks what they asked for. Also how a test reads any setting from +an environment variable whose name has changed. -The same file is in each test folder whose tests call the live cloud -service. There is no fallback to a skip, because a cloud test that passes -without calling the cloud has proved nothing. +The same file is in each test folder whose tests read a setting from the +environment. There is no fallback to a skip, because a cloud test that +passes without calling the cloud has proved nothing. """ import os +import sys # Tried in this order. _51DEGREES_RESOURCE_KEY is first because the Cloud # 51Did workflow sets it to each key in turn, and each product's @@ -39,18 +41,68 @@ "_51DEGREES_RESOURCE_KEY_BESPOKE", ) +# Deprecated. The name the resource key variable had before, read when +# none of RESOURCE_KEY_NAMES is set. +OLD_RESOURCE_KEY_NAME = "resource_key" + +# The old names already reported as deprecated, so each is reported once. +_reported_old_names = set() + ACCESSIBLE_PROPERTIES_URL = ( "https://cloud.51degrees.com/api/v4/accessibleproperties?resource=") CREATE_KEY_URL = "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=live_cloud.py&utm_term=resource-key-required" +def _report_old_name(name, old_name): + """Prints, once for each old name, that it is deprecated and which + name replaces it.""" + if old_name in _reported_old_names: + return + _reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated.""" + value = os.environ.get(name, "").strip() + if value: + return value + value = os.environ.get(old_name, "").strip() + if not value: + return None + _report_old_name(name, old_name) + return value + + +def is_set(name, old_name): + """Whether the environment variable name is set, whatever it holds, + which is how a switch is read. When it is not set, the name the + variable had before counts, and using it prints once that the old name + is deprecated.""" + if name in os.environ: + return True + if old_name not in os.environ: + return False + _report_old_name(name, old_name) + return True + + def find_resource_key(names=RESOURCE_KEY_NAMES): - """The first resource key set under any of the names, or None.""" + """The first resource key set under any of the names, or None. When + none of RESOURCE_KEY_NAMES is set, the deprecated old name is read.""" for name in names: value = os.environ.get(name, "").strip() if value: return value + if names is RESOURCE_KEY_NAMES: + return read_environment(names[0], OLD_RESOURCE_KEY_NAME) return None diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/tests/test_environment_names.py b/ip-intelligence/fiftyone_ipintelligence_examples/tests/test_environment_names.py new file mode 100644 index 00000000..27d3844d --- /dev/null +++ b/ip-intelligence/fiftyone_ipintelligence_examples/tests/test_environment_names.py @@ -0,0 +1,174 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""Checks that the example helper still reads each setting under the name +its environment variable had before, when the new name is not set, and +prints once that the old name is deprecated. These need no resource key. +""" + +import contextlib +import io +import os +import unittest +from unittest import mock + +import tempfile + +from fiftyone_ipintelligence_examples.example_utils import ExampleUtils + +# Each reader, with the new name of its variable and the old one. +PAIRS = { + "get_resource_key": ("_51DEGREES_RESOURCE_KEY", "resource_key"), + "get_license_key": ("_51DEGREES_LICENSE_KEY", "license_key"), + "get_cloud_endpoint": ("_51DEGREES_CLOUD_ENDPOINT", "cloud_endpoint"), +} + +# The data file variable, which find_file reads, under both names. +DATA_FILE = ("_51DEGREES_IPI_PATH", "IPINTELLIGENCEDATAFILE") + +# Each constant that names a variable, with the value it has to hold. The +# ones that held an old name hold it still. +CONSTANTS = { + "CLOUD_RESOURCE_KEY_ENV_VAR": "_51DEGREES_RESOURCE_KEY", + "RESOURCE_KEY_ENV_VAR": "resource_key", + "LICENSE_KEY_ENV_VAR": "_51DEGREES_LICENSE_KEY", + "LEGACY_LICENSE_KEY_ENV_VAR": "license_key", + "CLOUD_ENDPOINT_ENV_VAR": "_51DEGREES_CLOUD_ENDPOINT", + "ENDPOINT_ENV_VAR": "cloud_endpoint", + "IPI_PATH_ENV_VAR": "_51DEGREES_IPI_PATH", + "DATA_FILE_ENV_VAR": "IPINTELLIGENCEDATAFILE", +} + + +class OldNameTests(unittest.TestCase): + + def setUp(self): + # Each test starts with none of the names set and no old name + # reported yet, and what it sets is put back afterwards. + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + for pair in PAIRS.values(): + for name in pair: + os.environ.pop(name, None) + os.environ.pop(name.upper(), None) + reported = mock.patch.object( + ExampleUtils, "_ExampleUtils__reported_old_names", set()) + reported.start() + self.addCleanup(reported.stop) + + def printed(self, call): + """What a call returned and what it printed to the error stream.""" + errors = io.StringIO() + with contextlib.redirect_stderr(errors): + value = call() + return value, errors.getvalue() + + def test_each_setting_is_read_from_its_old_name(self): + for getter, (name, old_name) in PAIRS.items(): + with self.subTest(getter=getter): + os.environ[old_name] = "old-value" + value, errors = self.printed(getattr(ExampleUtils, getter)) + self.assertEqual("old-value", value) + self.assertIn( + old_name + " environment variable is deprecated", errors) + self.assertIn(name, errors) + del os.environ[old_name] + + def test_new_name_wins_and_nothing_is_printed(self): + for getter, (name, old_name) in PAIRS.items(): + with self.subTest(getter=getter): + os.environ[name] = "new-value" + os.environ[old_name] = "old-value" + value, errors = self.printed(getattr(ExampleUtils, getter)) + self.assertEqual("new-value", value) + self.assertEqual("", errors) + del os.environ[name] + del os.environ[old_name] + + def test_neither_name_gives_nothing(self): + for getter in PAIRS: + with self.subTest(getter=getter): + value, errors = self.printed(getattr(ExampleUtils, getter)) + self.assertFalse(value) + self.assertEqual("", errors) + + def test_old_name_is_reported_once(self): + getter, (name, old_name) = next(iter(PAIRS.items())) + os.environ[old_name] = "old-value" + + def twice(): + return (getattr(ExampleUtils, getter)(), + getattr(ExampleUtils, getter)()) + + value, errors = self.printed(twice) + self.assertEqual(("old-value", "old-value"), value) + self.assertEqual( + 1, errors.count(old_name + " environment variable is deprecated")) + + def test_constants_hold_the_old_and_the_new_names(self): + for constant, value in CONSTANTS.items(): + with self.subTest(constant=constant): + self.assertEqual(value, getattr(ExampleUtils, constant)) + + def test_key_and_endpoint_are_read_from_the_old_names_in_upper_case(self): + # Windows reads a variable name in any case, so the name the + # message gives is compared without regard to case. + for getter, upper_name in ( + ("get_resource_key", "RESOURCE_KEY"), + ("get_cloud_endpoint", "CLOUD_ENDPOINT")): + with self.subTest(getter=getter): + os.environ[upper_name] = "old-value" + value, errors = self.printed(getattr(ExampleUtils, getter)) + self.assertEqual("old-value", value) + self.assertIn( + upper_name.lower() + + " environment variable is deprecated", errors.lower()) + del os.environ[upper_name] + + def test_licence_key_is_not_read_in_upper_case(self): + # The licence key was only ever read under the lower case name. + # Windows reads a name in any case, so this holds elsewhere. + if os.name == "nt": + self.skipTest("Windows reads a variable name in any case") + os.environ["LICENSE_KEY"] = "old-value" + value, errors = self.printed(ExampleUtils.get_license_key) + self.assertFalse(value) + self.assertEqual("", errors) + + def test_data_file_is_found_under_the_old_name(self): + for name in DATA_FILE: + os.environ.pop(name, None) + with tempfile.TemporaryDirectory() as folder: + path = os.path.join(folder, "only-for-this-test.ipi") + with open(path, "wb") as data_file: + data_file.write(b"not a data file") + os.environ[DATA_FILE[1]] = path + value, errors = self.printed( + lambda: ExampleUtils.find_file("only-for-this-test.ipi")) + self.assertEqual(path, value) + self.assertIn( + DATA_FILE[1] + " environment variable is deprecated", errors) + + +if __name__ == "__main__": + unittest.main() diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/tests/test_examples_without_settings.py b/ip-intelligence/fiftyone_ipintelligence_examples/tests/test_examples_without_settings.py index 8d4cf2d3..e3cdf702 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/tests/test_examples_without_settings.py +++ b/ip-intelligence/fiftyone_ipintelligence_examples/tests/test_examples_without_settings.py @@ -42,7 +42,9 @@ from fiftyone_ipintelligence_examples.onpremise import performance_console # The settings the examples read, which a fresh clone does not have. -SETTINGS = ["_51DEGREES_LICENSE_KEY", ExampleUtils.DATA_FILE_ENV_VAR] +SETTINGS = [ + "_51DEGREES_LICENSE_KEY", "license_key", + ExampleUtils.IPI_PATH_ENV_VAR, ExampleUtils.DATA_FILE_ENV_VAR] # Options that keep the performance example short. They leave the data file # to the example. diff --git a/ip-intelligence/fiftyone_ipintelligence_examples/tox.ini b/ip-intelligence/fiftyone_ipintelligence_examples/tox.ini index c2c0088d..be89d1ca 100644 --- a/ip-intelligence/fiftyone_ipintelligence_examples/tox.ini +++ b/ip-intelligence/fiftyone_ipintelligence_examples/tox.ini @@ -38,9 +38,12 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_ipintelligence_examples {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_RESOURCE_KEY_BESPOKE _51DEGREES_LICENSE_KEY + license_key _51DEGREES_IPI_PATH + IPINTELLIGENCEDATAFILE TMPDIR TEMP TMP diff --git a/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/constants.py b/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/constants.py index dbffc9dd..3e5cdf7b 100644 --- a/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/constants.py +++ b/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/constants.py @@ -24,6 +24,8 @@ from fiftyone_ipintelligence_shared.example_constants import ASN_DATAFILE_NAME +from .live_cloud import read_environment + class TestConstants: @@ -31,11 +33,13 @@ class TestConstants: IPV6_ADDRESS = "2001:4860:4860::8888" # The data file can be supplied via the _51DEGREES_IPI_PATH - # environment variable. By default the Lite file downloaded to the + # environment variable, or under its deprecated old name. By default + # the Lite file downloaded to the # repository's assets folder by ci/fetch-assets.ps1 is used. @staticmethod def get_data_file_path(): - env = os.environ.get("_51DEGREES_IPI_PATH") + env = read_environment( + "_51DEGREES_IPI_PATH", "IPINTELLIGENCEDATAFILE") if env: return env return os.path.join( diff --git a/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/live_cloud.py b/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/live_cloud.py new file mode 100644 index 00000000..36bc26ad --- /dev/null +++ b/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/live_cloud.py @@ -0,0 +1,139 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""Where the tests that call the live 51Degrees cloud service find their +resource key, and what they fail with when there is none or when the +answer lacks what they asked for. Also how a test reads any setting from +an environment variable whose name has changed. + +The same file is in each test folder whose tests read a setting from the +environment. There is no fallback to a skip, because a cloud test that +passes without calling the cloud has proved nothing. +""" + +import os +import sys + +# Tried in this order. _51DEGREES_RESOURCE_KEY is first because the Cloud +# 51Did workflow sets it to each key in turn, and each product's +# integration test script sets it too. +RESOURCE_KEY_NAMES = ( + "_51DEGREES_RESOURCE_KEY", + "_51DEGREES_RESOURCE_KEY_BESPOKE", +) + +# Deprecated. The name the resource key variable had before, read when +# none of RESOURCE_KEY_NAMES is set. +OLD_RESOURCE_KEY_NAME = "resource_key" + +# The old names already reported as deprecated, so each is reported once. +_reported_old_names = set() + +ACCESSIBLE_PROPERTIES_URL = ( + "https://cloud.51degrees.com/api/v4/accessibleproperties?resource=") + +CREATE_KEY_URL = "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=live_cloud.py&utm_term=resource-key-required" + + +def _report_old_name(name, old_name): + """Prints, once for each old name, that it is deprecated and which + name replaces it.""" + if old_name in _reported_old_names: + return + _reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated.""" + value = os.environ.get(name, "").strip() + if value: + return value + value = os.environ.get(old_name, "").strip() + if not value: + return None + _report_old_name(name, old_name) + return value + + +def is_set(name, old_name): + """Whether the environment variable name is set, whatever it holds, + which is how a switch is read. When it is not set, the name the + variable had before counts, and using it prints once that the old name + is deprecated.""" + if name in os.environ: + return True + if old_name not in os.environ: + return False + _report_old_name(name, old_name) + return True + + +def find_resource_key(names=RESOURCE_KEY_NAMES): + """The first resource key set under any of the names, or None. When + none of RESOURCE_KEY_NAMES is set, the deprecated old name is read.""" + for name in names: + value = os.environ.get(name, "").strip() + if value: + return value + if names is RESOURCE_KEY_NAMES: + return read_environment(names[0], OLD_RESOURCE_KEY_NAME) + return None + + +def no_resource_key(product, asks_for, several=False, + names=RESOURCE_KEY_NAMES): + """What to fail with when no resource key is set. + + product names the product the key has to carry, with its article, as in + "the device product". asks_for says what the test asks the service for. + several is true where the message stands for a group of tests. + """ + if several: + opening = ("These tests call the live 51Degrees cloud service, so " + "they need a resource key") + asker = "the tests ask" + else: + opening = ("This test calls the live 51Degrees cloud service, so " + "it needs a resource key") + asker = "the test asks" + return ( + f"{opening} and there is nothing to fall back on. None of " + f"{', '.join(names)} is set, tried in that order. The key has to " + f"carry {product}, because {asker} the service for {asks_for}. To " + f"see the products a key carries, open {ACCESSIBLE_PROPERTIES_URL}. " + f"Create a key at {CREATE_KEY_URL}.") + + +def answered_without(thing, reason, product): + """What to fail with when the cloud answered without the thing asked + for. reason is the cloud's null reason, or None when it gave none.""" + return ( + f"The cloud answered without {thing} " + f"({reason or 'no reason given'}), which is what a key that does " + f"not carry {product} gets.") diff --git a/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/test_environment_names.py b/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/test_environment_names.py new file mode 100644 index 00000000..5fe9f2e5 --- /dev/null +++ b/ip-intelligence/fiftyone_ipintelligence_onpremise/tests/test_environment_names.py @@ -0,0 +1,66 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""Checks that the tests still read the data file path under the name its +environment variable had before, when the new name is not set. +""" + +import contextlib +import io +import os +import unittest +from unittest import mock + +from .constants import TestConstants + +NEW_NAME = "_51DEGREES_IPI_PATH" +OLD_NAME = "IPINTELLIGENCEDATAFILE" + + +class OldNameTests(unittest.TestCase): + + def setUp(self): + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + os.environ.pop(NEW_NAME, None) + os.environ.pop(OLD_NAME, None) + + def path(self): + with contextlib.redirect_stderr(io.StringIO()): + return TestConstants.get_data_file_path() + + def test_data_file_path_is_read_from_the_old_name(self): + os.environ[OLD_NAME] = "old.ipi" + self.assertEqual("old.ipi", self.path()) + + def test_new_name_wins(self): + os.environ[NEW_NAME] = "new.ipi" + os.environ[OLD_NAME] = "old.ipi" + self.assertEqual("new.ipi", self.path()) + + def test_neither_name_gives_the_lite_file(self): + self.assertTrue(self.path().endswith("51Degrees-LiteIpiV41.ipi")) + + +if __name__ == "__main__": + unittest.main() diff --git a/ip-intelligence/fiftyone_ipintelligence_onpremise/tox.ini b/ip-intelligence/fiftyone_ipintelligence_onpremise/tox.ini index b14580c8..8a13f223 100644 --- a/ip-intelligence/fiftyone_ipintelligence_onpremise/tox.ini +++ b/ip-intelligence/fiftyone_ipintelligence_onpremise/tox.ini @@ -35,8 +35,11 @@ commands = {[common]commands} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key _51DEGREES_IPI_PATH + IPINTELLIGENCEDATAFILE TMPDIR TEMP TMP diff --git a/ip-intelligence/fiftyone_ipintelligence_shared/src/fiftyone_ipintelligence_shared/key_utils.py b/ip-intelligence/fiftyone_ipintelligence_shared/src/fiftyone_ipintelligence_shared/key_utils.py index e4ea13a4..a6ef6753 100644 --- a/ip-intelligence/fiftyone_ipintelligence_shared/src/fiftyone_ipintelligence_shared/key_utils.py +++ b/ip-intelligence/fiftyone_ipintelligence_shared/src/fiftyone_ipintelligence_shared/key_utils.py @@ -21,10 +21,31 @@ # ********************************************************************* import os +import warnings class KeyUtils: - # A key from the environment variable of exactly that name. + # The upper case names already reported as deprecated, so each is + # reported once. + _reported_upper_case = set() + + # A key from the environment variable of that name. When that variable + # is unset or empty, the variable with the same name in upper case is + # read, which is deprecated and reported once with a FutureWarning. @staticmethod def get_named_key(key_name): - return os.environ.get(key_name) + value = os.environ.get(key_name) + upper = key_name.upper() + if value or upper == key_name: + return value + upper_value = os.environ.get(upper) + if not upper_value: + return value or upper_value + if upper not in KeyUtils._reported_upper_case: + KeyUtils._reported_upper_case.add(upper) + warnings.warn( + "Reading a key from the upper case form of its variable name " + "is deprecated. The key was read from {0} because {1} is not " + "set, so set {1} to the same value.".format(upper, key_name), + FutureWarning, stacklevel=2) + return upper_value diff --git a/ip-intelligence/fiftyone_ipintelligence_shared/tests/test_shared.py b/ip-intelligence/fiftyone_ipintelligence_shared/tests/test_shared.py index 834756dc..f3a5550a 100644 --- a/ip-intelligence/fiftyone_ipintelligence_shared/tests/test_shared.py +++ b/ip-intelligence/fiftyone_ipintelligence_shared/tests/test_shared.py @@ -46,6 +46,24 @@ def test_get_named_key(self): finally: del os.environ["_51DEGREES_TEST_NAMED_KEY"] + def test_get_named_key_reads_the_name_in_upper_case(self): + # The upper case form of a name is still read when the name as + # given is not set. + import os + os.environ.pop("_51degrees_test_named_key", None) + os.environ["_51DEGREES_TEST_NAMED_KEY"] = "value" + try: + self.assertEqual( + KeyUtils.get_named_key("_51degrees_test_named_key"), "value") + finally: + del os.environ["_51DEGREES_TEST_NAMED_KEY"] + + def test_get_named_key_without_a_key_gives_none(self): + import os + os.environ.pop("_51degrees_test_named_key", None) + os.environ.pop("_51DEGREES_TEST_NAMED_KEY", None) + self.assertIsNone(KeyUtils.get_named_key("_51degrees_test_named_key")) + if __name__ == "__main__": unittest.main() diff --git a/ip-intelligence/fiftyone_ipintelligence_shared/tox.ini b/ip-intelligence/fiftyone_ipintelligence_shared/tox.ini index c304f959..43f49189 100644 --- a/ip-intelligence/fiftyone_ipintelligence_shared/tox.ini +++ b/ip-intelligence/fiftyone_ipintelligence_shared/tox.ini @@ -22,7 +22,9 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_ipintelligence_shared {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key [testenv:pre-publish] package = skip diff --git a/ip-intelligence/fiftyone_ipintelligence_translation/tox.ini b/ip-intelligence/fiftyone_ipintelligence_translation/tox.ini index fabb28ba..e033217e 100644 --- a/ip-intelligence/fiftyone_ipintelligence_translation/tox.ini +++ b/ip-intelligence/fiftyone_ipintelligence_translation/tox.ini @@ -20,7 +20,9 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_ipintelligence_translation {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key [testenv:pre-publish] package = skip diff --git a/location/examples/cloud/gettingstarted.py b/location/examples/cloud/gettingstarted.py index fcd10ddf..143cc373 100644 --- a/location/examples/cloud/gettingstarted.py +++ b/location/examples/cloud/gettingstarted.py @@ -38,11 +38,33 @@ # You need to create a resource key at https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=examples-cloud-gettingstarted.py&utm_term=top # and paste it into the code, replacing !!YOUR_RESOURCE_KEY!! below. -# Alternatively, add a _51DEGREES_RESOURCE_KEY environment variable +# Alternatively, add a _51DEGREES_RESOURCE_KEY environment variable. +# resource_key is the old name of that variable. It is deprecated, and is +# read only when _51DEGREES_RESOURCE_KEY is not set. import os -if "_51DEGREES_RESOURCE_KEY" in os.environ: - resource_key = os.environ["_51DEGREES_RESOURCE_KEY"] -else: +import sys + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints that the old name is deprecated.""" + value = os.environ.get(name) + if value: + return value + value = os.environ.get(old_name) + if not value: + return None + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + return value + + +resource_key = read_environment("_51DEGREES_RESOURCE_KEY", "resource_key") +if not resource_key: resource_key = "!!YOUR_RESOURCE_KEY!!" if resource_key == "!!YOUR_RESOURCE_KEY!!": diff --git a/location/examples/cloud/web.py b/location/examples/cloud/web.py index a5db8558..f893824f 100644 --- a/location/examples/cloud/web.py +++ b/location/examples/cloud/web.py @@ -38,11 +38,33 @@ # You need to create a resource key at https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=examples-cloud-web.py&utm_term=top # and paste it into the code, replacing !!YOUR_RESOURCE_KEY!! below. -# Alternatively, add a _51DEGREES_RESOURCE_KEY environment variable +# Alternatively, add a _51DEGREES_RESOURCE_KEY environment variable. +# resource_key is the old name of that variable. It is deprecated, and is +# read only when _51DEGREES_RESOURCE_KEY is not set. import os -if "_51DEGREES_RESOURCE_KEY" in os.environ: - resource_key = os.environ["_51DEGREES_RESOURCE_KEY"] -else: +import sys + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints that the old name is deprecated.""" + value = os.environ.get(name) + if value: + return value + value = os.environ.get(old_name) + if not value: + return None + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + return value + + +resource_key = read_environment("_51DEGREES_RESOURCE_KEY", "resource_key") +if not resource_key: resource_key = "!!YOUR_RESOURCE_KEY!!" if resource_key == "!!YOUR_RESOURCE_KEY!!": diff --git a/location/tests/live_cloud.py b/location/tests/live_cloud.py new file mode 100644 index 00000000..36bc26ad --- /dev/null +++ b/location/tests/live_cloud.py @@ -0,0 +1,139 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""Where the tests that call the live 51Degrees cloud service find their +resource key, and what they fail with when there is none or when the +answer lacks what they asked for. Also how a test reads any setting from +an environment variable whose name has changed. + +The same file is in each test folder whose tests read a setting from the +environment. There is no fallback to a skip, because a cloud test that +passes without calling the cloud has proved nothing. +""" + +import os +import sys + +# Tried in this order. _51DEGREES_RESOURCE_KEY is first because the Cloud +# 51Did workflow sets it to each key in turn, and each product's +# integration test script sets it too. +RESOURCE_KEY_NAMES = ( + "_51DEGREES_RESOURCE_KEY", + "_51DEGREES_RESOURCE_KEY_BESPOKE", +) + +# Deprecated. The name the resource key variable had before, read when +# none of RESOURCE_KEY_NAMES is set. +OLD_RESOURCE_KEY_NAME = "resource_key" + +# The old names already reported as deprecated, so each is reported once. +_reported_old_names = set() + +ACCESSIBLE_PROPERTIES_URL = ( + "https://cloud.51degrees.com/api/v4/accessibleproperties?resource=") + +CREATE_KEY_URL = "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=live_cloud.py&utm_term=resource-key-required" + + +def _report_old_name(name, old_name): + """Prints, once for each old name, that it is deprecated and which + name replaces it.""" + if old_name in _reported_old_names: + return + _reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated.""" + value = os.environ.get(name, "").strip() + if value: + return value + value = os.environ.get(old_name, "").strip() + if not value: + return None + _report_old_name(name, old_name) + return value + + +def is_set(name, old_name): + """Whether the environment variable name is set, whatever it holds, + which is how a switch is read. When it is not set, the name the + variable had before counts, and using it prints once that the old name + is deprecated.""" + if name in os.environ: + return True + if old_name not in os.environ: + return False + _report_old_name(name, old_name) + return True + + +def find_resource_key(names=RESOURCE_KEY_NAMES): + """The first resource key set under any of the names, or None. When + none of RESOURCE_KEY_NAMES is set, the deprecated old name is read.""" + for name in names: + value = os.environ.get(name, "").strip() + if value: + return value + if names is RESOURCE_KEY_NAMES: + return read_environment(names[0], OLD_RESOURCE_KEY_NAME) + return None + + +def no_resource_key(product, asks_for, several=False, + names=RESOURCE_KEY_NAMES): + """What to fail with when no resource key is set. + + product names the product the key has to carry, with its article, as in + "the device product". asks_for says what the test asks the service for. + several is true where the message stands for a group of tests. + """ + if several: + opening = ("These tests call the live 51Degrees cloud service, so " + "they need a resource key") + asker = "the tests ask" + else: + opening = ("This test calls the live 51Degrees cloud service, so " + "it needs a resource key") + asker = "the test asks" + return ( + f"{opening} and there is nothing to fall back on. None of " + f"{', '.join(names)} is set, tried in that order. The key has to " + f"carry {product}, because {asker} the service for {asks_for}. To " + f"see the products a key carries, open {ACCESSIBLE_PROPERTIES_URL}. " + f"Create a key at {CREATE_KEY_URL}.") + + +def answered_without(thing, reason, product): + """What to fail with when the cloud answered without the thing asked + for. reason is the cloud's null reason, or None when it gave none.""" + return ( + f"The cloud answered without {thing} " + f"({reason or 'no reason given'}), which is what a key that does " + f"not carry {product} gets.") diff --git a/location/tests/test_environment_names.py b/location/tests/test_environment_names.py new file mode 100644 index 00000000..f3ad6f0a --- /dev/null +++ b/location/tests/test_environment_names.py @@ -0,0 +1,114 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""Checks that the two cloud examples still read the resource key under +the name its environment variable had before, when the new name is not set. + +Importing an example runs it, which calls the cloud service, so the +function each example reads its key with is taken out of the example's +source and run on its own. +""" + +import ast +import contextlib +import io +import os +import sys +import unittest +from pathlib import Path +from unittest import mock + +EXAMPLES = Path(__file__).resolve().parent.parent / "examples" / "cloud" + +NEW_NAME = "_51DEGREES_RESOURCE_KEY" +OLD_NAME = "resource_key" + + +def reader(example): + """The read_environment function of an example, or None when the + example has none.""" + source = (EXAMPLES / example).read_text(encoding="utf-8") + for node in ast.parse(source).body: + if isinstance(node, ast.FunctionDef) \ + and node.name == "read_environment": + module = ast.Module(body=[node], type_ignores=[]) + names = {"os": os, "sys": sys} + exec(compile(module, example, "exec"), names) + return names["read_environment"] + return None + + +class OldNameTests(unittest.TestCase): + + def setUp(self): + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + os.environ.pop(NEW_NAME, None) + os.environ.pop(OLD_NAME, None) + + def read(self, example): + """What the example reads for its resource key, and what it prints + to the error stream.""" + read_environment = reader(example) + self.assertIsNotNone( + read_environment, + example + " has no read_environment function, so it does not " + "read the old name") + # The example has to read the key through the function. + self.assertIn( + 'read_environment("' + NEW_NAME + '", "' + OLD_NAME + '")', + (EXAMPLES / example).read_text(encoding="utf-8")) + errors = io.StringIO() + with contextlib.redirect_stderr(errors): + value = read_environment(NEW_NAME, OLD_NAME) + return value, errors.getvalue() + + def test_resource_key_is_read_from_the_old_name(self): + for example in ("gettingstarted.py", "web.py"): + with self.subTest(example=example): + os.environ[OLD_NAME] = "old-key" + value, errors = self.read(example) + self.assertEqual("old-key", value) + self.assertIn( + OLD_NAME + " environment variable is deprecated", errors) + self.assertIn(NEW_NAME, errors) + + def test_new_name_wins_and_nothing_is_printed(self): + for example in ("gettingstarted.py", "web.py"): + with self.subTest(example=example): + os.environ[NEW_NAME] = "new-key" + os.environ[OLD_NAME] = "old-key" + value, errors = self.read(example) + self.assertEqual("new-key", value) + self.assertEqual("", errors) + + def test_neither_name_gives_none(self): + for example in ("gettingstarted.py", "web.py"): + with self.subTest(example=example): + value, errors = self.read(example) + self.assertIsNone(value) + self.assertEqual("", errors) + + +if __name__ == "__main__": + unittest.main() diff --git a/location/tests/test_examples.py b/location/tests/test_examples.py index c1e6c276..d01a5bee 100644 --- a/location/tests/test_examples.py +++ b/location/tests/test_examples.py @@ -25,10 +25,12 @@ import unittest from pathlib import Path +from .live_cloud import read_environment + class LocationExampleTests(unittest.TestCase): def test_cloud_getting_started(self): - if "_51DEGREES_RESOURCE_KEY" in os.environ: + if read_environment("_51DEGREES_RESOURCE_KEY", "resource_key"): # this is required to import the example code, # since pytest sets sys.path to the test directory sys.path.append(str(Path(__file__).parent.parent.absolute() / "examples")) diff --git a/location/tests/test_properties51D.py b/location/tests/test_properties51D.py index 16c347c8..a31468c2 100644 --- a/location/tests/test_properties51D.py +++ b/location/tests/test_properties51D.py @@ -25,13 +25,13 @@ from fiftyone_location.location_pipelinebuilder import LocationPipelineBuilder from .test_helper import * +from .live_cloud import read_environment location_LAT = '51.4578261'; location_LON = '-0.975922996290084'; -if "_51DEGREES_RESOURCE_KEY" in os.environ: - resource_key = os.environ["_51DEGREES_RESOURCE_KEY"] -else: +resource_key = read_environment("_51DEGREES_RESOURCE_KEY", "resource_key") +if not resource_key: raise Exception("To run the cloud tests, please set a valid 51Degrees " "cloud resource key as the _51DEGREES_RESOURCE_KEY environment variable.") diff --git a/location/tox.ini b/location/tox.ini index e74b3a9f..dbb7b77b 100644 --- a/location/tox.ini +++ b/location/tox.ini @@ -17,7 +17,9 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_location {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key [testenv:pre-publish] package = skip diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/examples/src/fiftyone_pipeline_examples/example_utils.py b/pipeline/fiftyone_pipeline_cloudrequestengine/examples/src/fiftyone_pipeline_examples/example_utils.py index 3614ec68..e1534c4d 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/examples/src/fiftyone_pipeline_examples/example_utils.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/examples/src/fiftyone_pipeline_examples/example_utils.py @@ -26,6 +26,7 @@ """ import os +import sys class ExampleUtils: @@ -35,38 +36,89 @@ class ExampleUtils: # "_51DEGREES_RESOURCE_KEY". RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY" + # Deprecated. Use RESOURCE_KEY_ENV_VAR. The name the resource key + # variable had before, read when RESOURCE_KEY_ENV_VAR is not set. + LEGACY_RESOURCE_KEY_ENV_VAR = "resource_key" + # The environment variable the examples read a licence key from, to use # in place of a resource key. LICENSE_KEY_ENV_VAR = "_51DEGREES_LICENSE_KEY" + # Deprecated. Use LICENSE_KEY_ENV_VAR. The name the licence key + # variable had before, read when LICENSE_KEY_ENV_VAR is not set. + LEGACY_LICENSE_KEY_ENV_VAR = "license_key" + # The environment variable naming a cloud service other than the public # one, for example one hosted privately. The value includes the api/v4 # path, as in https://cloud.51degrees.com/api/v4/. - ENDPOINT_ENV_VAR = "_51DEGREES_CLOUD_ENDPOINT" + CLOUD_ENDPOINT_ENV_VAR = "_51DEGREES_CLOUD_ENDPOINT" + + # Deprecated. Use CLOUD_ENDPOINT_ENV_VAR. The name that variable had + # before, read when CLOUD_ENDPOINT_ENV_VAR is not set. + ENDPOINT_ENV_VAR = "cloud_endpoint" # The environment variable holding the origin to send with each request # to the cloud service, for a resource key limited to particular # domains, which only answers a request whose origin is one of them. - ORIGIN_ENV_VAR = "_51DEGREES_CLOUD_REQUEST_ORIGIN" + CLOUD_REQUEST_ORIGIN_ENV_VAR = "_51DEGREES_CLOUD_REQUEST_ORIGIN" + + # Deprecated. Use CLOUD_REQUEST_ORIGIN_ENV_VAR. The name that variable + # had before, read when CLOUD_REQUEST_ORIGIN_ENV_VAR is not set. + ORIGIN_ENV_VAR = "cloud_request_origin" # The text shown for a value the results do not hold. UNKNOWN = "Unknown" @staticmethod def get_resource_key(): - return os.environ.get(ExampleUtils.RESOURCE_KEY_ENV_VAR) or "" + return ExampleUtils.__get_env_variable( + ExampleUtils.RESOURCE_KEY_ENV_VAR, + ExampleUtils.LEGACY_RESOURCE_KEY_ENV_VAR) @staticmethod def get_license_key(): - return os.environ.get(ExampleUtils.LICENSE_KEY_ENV_VAR) or "" + return ExampleUtils.__get_env_variable( + ExampleUtils.LICENSE_KEY_ENV_VAR, + ExampleUtils.LEGACY_LICENSE_KEY_ENV_VAR) @staticmethod def get_cloud_endpoint(): - return os.environ.get(ExampleUtils.ENDPOINT_ENV_VAR) or "" + return ExampleUtils.__get_env_variable( + ExampleUtils.CLOUD_ENDPOINT_ENV_VAR, + ExampleUtils.ENDPOINT_ENV_VAR) @staticmethod def get_cloud_request_origin(): - return os.environ.get(ExampleUtils.ORIGIN_ENV_VAR) or "" + return ExampleUtils.__get_env_variable( + ExampleUtils.CLOUD_REQUEST_ORIGIN_ENV_VAR, + ExampleUtils.ORIGIN_ENV_VAR) + + # The old names already reported as deprecated, so each is reported + # once. + __reported_old_names = set() + + @staticmethod + def __get_env_variable(name, old_name): + """! + The value of the environment variable name, or an empty string. + When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated. + """ + + value = os.environ.get(name) + if value: + return value + value = os.environ.get(old_name) + if not value: + return "" + if old_name not in ExampleUtils.__reported_old_names: + ExampleUtils.__reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + return value @staticmethod def name_values(section, property_names): diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests/live_cloud.py b/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests/live_cloud.py index 23eb9f4f..36bc26ad 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests/live_cloud.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests/live_cloud.py @@ -22,14 +22,16 @@ """Where the tests that call the live 51Degrees cloud service find their resource key, and what they fail with when there is none or when the -answer lacks what they asked for. +answer lacks what they asked for. Also how a test reads any setting from +an environment variable whose name has changed. -The same file is in each test folder whose tests call the live cloud -service. There is no fallback to a skip, because a cloud test that passes -without calling the cloud has proved nothing. +The same file is in each test folder whose tests read a setting from the +environment. There is no fallback to a skip, because a cloud test that +passes without calling the cloud has proved nothing. """ import os +import sys # Tried in this order. _51DEGREES_RESOURCE_KEY is first because the Cloud # 51Did workflow sets it to each key in turn, and each product's @@ -39,18 +41,68 @@ "_51DEGREES_RESOURCE_KEY_BESPOKE", ) +# Deprecated. The name the resource key variable had before, read when +# none of RESOURCE_KEY_NAMES is set. +OLD_RESOURCE_KEY_NAME = "resource_key" + +# The old names already reported as deprecated, so each is reported once. +_reported_old_names = set() + ACCESSIBLE_PROPERTIES_URL = ( "https://cloud.51degrees.com/api/v4/accessibleproperties?resource=") CREATE_KEY_URL = "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=live_cloud.py&utm_term=resource-key-required" +def _report_old_name(name, old_name): + """Prints, once for each old name, that it is deprecated and which + name replaces it.""" + if old_name in _reported_old_names: + return + _reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated.""" + value = os.environ.get(name, "").strip() + if value: + return value + value = os.environ.get(old_name, "").strip() + if not value: + return None + _report_old_name(name, old_name) + return value + + +def is_set(name, old_name): + """Whether the environment variable name is set, whatever it holds, + which is how a switch is read. When it is not set, the name the + variable had before counts, and using it prints once that the old name + is deprecated.""" + if name in os.environ: + return True + if old_name not in os.environ: + return False + _report_old_name(name, old_name) + return True + + def find_resource_key(names=RESOURCE_KEY_NAMES): - """The first resource key set under any of the names, or None.""" + """The first resource key set under any of the names, or None. When + none of RESOURCE_KEY_NAMES is set, the deprecated old name is read.""" for name in names: value = os.environ.get(name, "").strip() if value: return value + if names is RESOURCE_KEY_NAMES: + return read_environment(names[0], OLD_RESOURCE_KEY_NAME) return None diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests/test_example_utils.py b/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests/test_example_utils.py index c42292e2..1b6cc187 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests/test_example_utils.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests/test_example_utils.py @@ -30,6 +30,70 @@ from fiftyone_pipeline_examples.cloud.engines import WeightedCloudEngine from fiftyone_pipeline_examples.example_utils import ExampleUtils +import pytest + +# Each reader of a setting, with the new name of its variable and the old +# one, which is still read when the new name is not set. +OLD_NAMES = [ + ("get_resource_key", "_51DEGREES_RESOURCE_KEY", "resource_key"), + ("get_license_key", "_51DEGREES_LICENSE_KEY", "license_key"), + ("get_cloud_endpoint", "_51DEGREES_CLOUD_ENDPOINT", "cloud_endpoint"), + ("get_cloud_request_origin", "_51DEGREES_CLOUD_REQUEST_ORIGIN", + "cloud_request_origin"), +] + + +@pytest.fixture +def no_settings(monkeypatch): + """None of the settings in the environment, and no old name reported + as deprecated yet.""" + for _, name, old_name in OLD_NAMES: + monkeypatch.delenv(name, raising=False) + monkeypatch.delenv(old_name, raising=False) + monkeypatch.setattr( + ExampleUtils, "_ExampleUtils__reported_old_names", set()) + return monkeypatch + + +@pytest.mark.parametrize("getter, name, old_name", OLD_NAMES) +def test_setting_is_read_from_its_old_name_and_reported_once( + getter, name, old_name, no_settings, capsys): + no_settings.setenv(old_name, "old-value") + assert getattr(ExampleUtils, getter)() == "old-value" + assert getattr(ExampleUtils, getter)() == "old-value" + errors = capsys.readouterr().err + assert errors.count( + old_name + " environment variable is deprecated") == 1 + assert name in errors + + +@pytest.mark.parametrize("getter, name, old_name", OLD_NAMES) +def test_new_name_wins_and_nothing_is_printed( + getter, name, old_name, no_settings, capsys): + no_settings.setenv(name, "new-value") + no_settings.setenv(old_name, "old-value") + assert getattr(ExampleUtils, getter)() == "new-value" + assert capsys.readouterr().err == "" + + +@pytest.mark.parametrize("getter, name, old_name", OLD_NAMES) +def test_neither_name_gives_an_empty_string( + getter, name, old_name, no_settings, capsys): + assert getattr(ExampleUtils, getter)() == "" + assert capsys.readouterr().err == "" + + +def test_constants_hold_the_old_and_the_new_names(): + assert ExampleUtils.RESOURCE_KEY_ENV_VAR == "_51DEGREES_RESOURCE_KEY" + assert ExampleUtils.LEGACY_RESOURCE_KEY_ENV_VAR == "resource_key" + assert ExampleUtils.LICENSE_KEY_ENV_VAR == "_51DEGREES_LICENSE_KEY" + assert ExampleUtils.LEGACY_LICENSE_KEY_ENV_VAR == "license_key" + assert ExampleUtils.CLOUD_ENDPOINT_ENV_VAR == "_51DEGREES_CLOUD_ENDPOINT" + assert ExampleUtils.ENDPOINT_ENV_VAR == "cloud_endpoint" + assert (ExampleUtils.CLOUD_REQUEST_ORIGIN_ENV_VAR + == "_51DEGREES_CLOUD_REQUEST_ORIGIN") + assert ExampleUtils.ORIGIN_ENV_VAR == "cloud_request_origin" + class Results: """Stands in for an engine's results, holding one value per property.""" diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tox.ini b/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tox.ini index fbd5e095..cd1bd54a 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tox.ini +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/examples/tox.ini @@ -37,10 +37,15 @@ commands = # examples for a resource key limited to particular domains. pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_RESOURCE_KEY_BESPOKE _51DEGREES_LICENSE_KEY + license_key _51DEGREES_CLOUD_ENDPOINT + FOD_CLOUD_API_URL + cloud_endpoint _51DEGREES_CLOUD_REQUEST_ORIGIN + cloud_request_origin [testenv:pre-publish] deps = diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md index de740e3b..e123445a 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/readme.md @@ -63,7 +63,11 @@ and the rules in The engine calls `https://cloud.51degrees.com/api/v4/` unless told otherwise. Set the `_51DEGREES_CLOUD_ENDPOINT` environment variable, or pass `cloud_endpoint` in the engine settings, to the API base of another -host including the `/api/v4/` segment. A host other than +host including the `/api/v4/` segment. `FOD_CLOUD_API_URL`, the old name +of that variable, is deprecated. The engine reads it only when +`_51DEGREES_CLOUD_ENDPOINT` is not set, and warns once that the old name +was used. `Constants.FOD_CLOUD_API_URL` is deprecated as well, and +`Constants.CLOUD_ENDPOINT` replaces it. A host other than cloud.51degrees.com would be used to (a) use an on premise web server, or (b) use a privately hosted version of the 51Degrees cloud for performance reasons. This is the private hosting option of the cloud diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py index 5dfc6bc0..ca3146b4 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py @@ -24,6 +24,7 @@ import json import os +import threading import warnings from functools import cached_property from json import JSONDecodeError @@ -48,6 +49,50 @@ from urllib.parse import urlencode +# Whether the old end point variable has been reported as deprecated, so +# that it is reported once however many engines are built. +_old_endpoint_reported = False +_old_endpoint_lock = threading.Lock() + + +def _endpoint_from_environment(): + + """! + The cloud end point the environment gives, or None when it gives none. + + Constants.CLOUD_ENDPOINT is read first. When it is unset or empty the + variable Constants.FOD_CLOUD_API_URL names is read, which is the old + name, and the first use of it is reported as deprecated. The report is + a FutureWarning, which Python shows by default, because the people who + set the variable are the ones running the application. + + @rtype: string + @return: the end point, or None + """ + + global _old_endpoint_reported + + endpoint = os.environ.get(Constants.CLOUD_ENDPOINT) + if endpoint: + return endpoint + + old_name = Constants.FOD_CLOUD_API_URL + endpoint = os.environ.get(old_name) + if not endpoint: + return None + + with _old_endpoint_lock: + report = not _old_endpoint_reported + _old_endpoint_reported = True + if report: + # The third frame is the code that built the engine. + warnings.warn( + Constants.MESSAGE_OLD_ENDPOINT_VARIABLE.format( + old_name, Constants.CLOUD_ENDPOINT), + FutureWarning, stacklevel=3) + return endpoint + + # Engine that makes a call to the 51Degrees cloud service # Returns raw JSON as a "cloud" property under "cloud" datakey class CloudRequestEngine(Engine): @@ -106,7 +151,8 @@ def __init__(self, settings = {}): self.reported_not_covered = set() # The endpoint is the cloud_endpoint setting, then the - # _51DEGREES_CLOUD_ENDPOINT environment variable, then + # _51DEGREES_CLOUD_ENDPOINT environment variable, then that + # variable's deprecated old name FOD_CLOUD_API_URL, then # cloud.51degrees.com. A host other than cloud.51degrees.com # would be used to (a) use an on premise web server, or (b) use # a privately hosted version of the 51Degrees cloud for @@ -116,8 +162,8 @@ def __init__(self, settings = {}): if "cloud_endpoint" in settings: self.baseURL = settings["cloud_endpoint"] else: - self.baseURL = os.environ.get(Constants.CLOUD_ENDPOINT) - if self.baseURL is None or (self.baseURL is not None and self.baseURL == ""): + self.baseURL = _endpoint_from_environment() + if self.baseURL is None: self.baseURL = Constants.BASE_URL_DEFAULT # Make sure if baseURL does not end with '/', one will be appended diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py index 60bd4d24..fa7c879c 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py @@ -23,6 +23,21 @@ class Constants: # Environment variable to set cloud end point CLOUD_ENDPOINT = "_51DEGREES_CLOUD_ENDPOINT" + + # @deprecated Use Constants.CLOUD_ENDPOINT. This holds the old name of + # the cloud end point environment variable, which the engine reads + # through this constant when CLOUD_ENDPOINT is not set, so code that + # passes this constant to the environment reads and sets the old + # variable. + FOD_CLOUD_API_URL = "FOD_CLOUD_API_URL" + + # Message when the cloud end point came from the old environment + # variable. The first value is the old name and the second the new one. + MESSAGE_OLD_ENDPOINT_VARIABLE = ( + "The {0} environment variable is deprecated and {1} replaces it. " + "The cloud endpoint was read from {0} because {1} is not set, so " + "set {1} to the same value.") + # Default cloud end point BASE_URL_DEFAULT = "https://cloud.51degrees.com/api/v4/" diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/live_cloud.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/live_cloud.py new file mode 100644 index 00000000..36bc26ad --- /dev/null +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/live_cloud.py @@ -0,0 +1,139 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""Where the tests that call the live 51Degrees cloud service find their +resource key, and what they fail with when there is none or when the +answer lacks what they asked for. Also how a test reads any setting from +an environment variable whose name has changed. + +The same file is in each test folder whose tests read a setting from the +environment. There is no fallback to a skip, because a cloud test that +passes without calling the cloud has proved nothing. +""" + +import os +import sys + +# Tried in this order. _51DEGREES_RESOURCE_KEY is first because the Cloud +# 51Did workflow sets it to each key in turn, and each product's +# integration test script sets it too. +RESOURCE_KEY_NAMES = ( + "_51DEGREES_RESOURCE_KEY", + "_51DEGREES_RESOURCE_KEY_BESPOKE", +) + +# Deprecated. The name the resource key variable had before, read when +# none of RESOURCE_KEY_NAMES is set. +OLD_RESOURCE_KEY_NAME = "resource_key" + +# The old names already reported as deprecated, so each is reported once. +_reported_old_names = set() + +ACCESSIBLE_PROPERTIES_URL = ( + "https://cloud.51degrees.com/api/v4/accessibleproperties?resource=") + +CREATE_KEY_URL = "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=live_cloud.py&utm_term=resource-key-required" + + +def _report_old_name(name, old_name): + """Prints, once for each old name, that it is deprecated and which + name replaces it.""" + if old_name in _reported_old_names: + return + _reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated.""" + value = os.environ.get(name, "").strip() + if value: + return value + value = os.environ.get(old_name, "").strip() + if not value: + return None + _report_old_name(name, old_name) + return value + + +def is_set(name, old_name): + """Whether the environment variable name is set, whatever it holds, + which is how a switch is read. When it is not set, the name the + variable had before counts, and using it prints once that the old name + is deprecated.""" + if name in os.environ: + return True + if old_name not in os.environ: + return False + _report_old_name(name, old_name) + return True + + +def find_resource_key(names=RESOURCE_KEY_NAMES): + """The first resource key set under any of the names, or None. When + none of RESOURCE_KEY_NAMES is set, the deprecated old name is read.""" + for name in names: + value = os.environ.get(name, "").strip() + if value: + return value + if names is RESOURCE_KEY_NAMES: + return read_environment(names[0], OLD_RESOURCE_KEY_NAME) + return None + + +def no_resource_key(product, asks_for, several=False, + names=RESOURCE_KEY_NAMES): + """What to fail with when no resource key is set. + + product names the product the key has to carry, with its article, as in + "the device product". asks_for says what the test asks the service for. + several is true where the message stands for a group of tests. + """ + if several: + opening = ("These tests call the live 51Degrees cloud service, so " + "they need a resource key") + asker = "the tests ask" + else: + opening = ("This test calls the live 51Degrees cloud service, so " + "it needs a resource key") + asker = "the test asks" + return ( + f"{opening} and there is nothing to fall back on. None of " + f"{', '.join(names)} is set, tried in that order. The key has to " + f"carry {product}, because {asker} the service for {asks_for}. To " + f"see the products a key carries, open {ACCESSIBLE_PROPERTIES_URL}. " + f"Create a key at {CREATE_KEY_URL}.") + + +def answered_without(thing, reason, product): + """What to fail with when the cloud answered without the thing asked + for. reason is the cloud's null reason, or None when it gave none.""" + return ( + f"The cloud answered without {thing} " + f"({reason or 'no reason given'}), which is what a key that does " + f"not carry {product} gets.") diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine.py index dff83016..b88ed79d 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine.py @@ -30,10 +30,13 @@ from fiftyone_pipeline_cloudrequestengine.cloudengine import CloudEngine from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder +from .live_cloud import read_environment + class CloudEngineTests(unittest.TestCase): def setUp(self): - self.resource_key = os.environ.get("_51DEGREES_RESOURCE_KEY") + self.resource_key = read_environment( + "_51DEGREES_RESOURCE_KEY", "resource_key") self.assertIsNotNone( self.resource_key, diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py index 8d4bf357..1078da4f 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py @@ -22,8 +22,10 @@ import os import warnings +from unittest import mock from parameterized import parameterized +from fiftyone_pipeline_cloudrequestengine import cloudrequestengine from fiftyone_pipeline_cloudrequestengine.cloudrequestexception import CloudRequestException from fiftyone_pipeline_cloudrequestengine.cloudrequestengine import CloudRequestEngine from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder @@ -45,10 +47,50 @@ class TestCloudRequestEngine(CloudRequestEngineTestsBase): testResourceKey = "resource_key" testEndPoint = "http://testEndPoint/" testEnvVarEndPoint = "http://testEnvVarEndPoint/" + testOldEnvVarEndPoint = "http://testOldEnvVarEndPoint/" + + # The name the endpoint environment variable had before + # Constants.CLOUD_ENDPOINT, written out so that these tests do not + # depend on the deprecated constant that holds it. + oldEndPointVariable = "FOD_CLOUD_API_URL" def setUp(self): - if os.environ.get(Constants.CLOUD_ENDPOINT) is not None: - del os.environ[Constants.CLOUD_ENDPOINT] + # Each test starts with neither name of the endpoint variable set + # and with the old name not yet reported as deprecated, and what + # it sets is put back afterwards. + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + os.environ.pop(Constants.CLOUD_ENDPOINT, None) + os.environ.pop(self.oldEndPointVariable, None) + reported = mock.patch.object( + cloudrequestengine, "_old_endpoint_reported", False) + reported.start() + self.addCleanup(reported.stop) + + def new_engine(self, **settings): + """! + A cloud request engine with the test resource key, a mock HTTP + client and any other settings given. + """ + + return CloudRequestEngine({ + "resource_key": TestCloudRequestEngine.testResourceKey, + "http_client": self.mock_http(), + **settings + }) + + def old_name_reports(self, caught): + """! + The warnings in a recorded list that say the old endpoint variable + is deprecated and name the one that replaces it. + """ + + return [ + warning for warning in caught + if issubclass(warning.category, FutureWarning) + and self.oldEndPointVariable in str(warning.message) + and Constants.CLOUD_ENDPOINT in str(warning.message)] def testConfigEndPoint_ExplicitSetting(self): """! @@ -88,6 +130,123 @@ def testConfigEndPoint_DefaultSetting(self): }) self.assertEqual(Constants.BASE_URL_DEFAULT, cloud_request_engine.baseURL) + def testConfigEndPoint_OldEnvironmentVariable(self): + """! + Check that the old name of the endpoint environment variable is + still read when the new name is not set + """ + + os.environ[self.oldEndPointVariable] = self.testOldEnvVarEndPoint + with warnings.catch_warnings(): + warnings.simplefilter("ignore", FutureWarning) + engine = self.new_engine() + self.assertEqual(self.testOldEnvVarEndPoint, engine.baseURL) + + def testConfigEndPoint_OldEnvironmentVariable_NewNameEmpty(self): + """! + Check that the old name is read when the new name is set to an + empty value, which counts as not set + """ + + os.environ[Constants.CLOUD_ENDPOINT] = "" + os.environ[self.oldEndPointVariable] = self.testOldEnvVarEndPoint + with warnings.catch_warnings(): + warnings.simplefilter("ignore", FutureWarning) + engine = self.new_engine() + self.assertEqual(self.testOldEnvVarEndPoint, engine.baseURL) + + def testConfigEndPoint_OldEnvironmentVariable_Empty(self): + """! + Check that the default is used when the old name is set to an + empty value and the new name is not set, with nothing reported + """ + + os.environ[self.oldEndPointVariable] = "" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + engine = self.new_engine() + self.assertEqual(Constants.BASE_URL_DEFAULT, engine.baseURL) + self.assertEqual([], self.old_name_reports(caught)) + + def testConfigEndPoint_NewEnvironmentVariableWins(self): + """! + Check that the new name wins when both names are set, and that the + old name is not reported because it was not used + """ + + os.environ[Constants.CLOUD_ENDPOINT] = self.testEnvVarEndPoint + os.environ[self.oldEndPointVariable] = self.testOldEnvVarEndPoint + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + engine = self.new_engine() + self.assertEqual(self.testEnvVarEndPoint, engine.baseURL) + self.assertEqual([], self.old_name_reports(caught)) + + def testConfigEndPoint_ExplicitSettingOverOldEnvironmentVariable(self): + """! + Check that the cloud_endpoint setting wins over the old name, and + that the old name is not reported because it was not used + """ + + os.environ[self.oldEndPointVariable] = self.testOldEnvVarEndPoint + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + engine = self.new_engine(cloud_endpoint=self.testEndPoint) + self.assertEqual(self.testEndPoint, engine.baseURL) + self.assertEqual([], self.old_name_reports(caught)) + + def testConfigEndPoint_OldEnvironmentVariableReportedOnce(self): + """! + Check that using the old name is reported as deprecated once, + naming the new name, however many engines are built + """ + + os.environ[self.oldEndPointVariable] = self.testOldEnvVarEndPoint + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + first = self.new_engine() + second = self.new_engine() + self.assertEqual(self.testOldEnvVarEndPoint, first.baseURL) + self.assertEqual(self.testOldEnvVarEndPoint, second.baseURL) + self.assertEqual(1, len(self.old_name_reports(caught))) + + def testOldEndPointConstant(self): + """! + Check that the constant the old name was held in is still there + with its old value, as a plain attribute of its class, so that + every way of reading it gives the old name + """ + + self.assertEqual( + self.oldEndPointVariable, Constants.FOD_CLOUD_API_URL) + self.assertEqual( + self.oldEndPointVariable, + vars(Constants).get("FOD_CLOUD_API_URL")) + self.assertIn("FOD_CLOUD_API_URL", dir(Constants)) + + def testConfigEndPoint_VariableTheOldConstantNames(self): + """! + Check that the engine reads the old name through the constant, as + it did before, so that code which points the constant at another + variable still decides what is read + """ + + os.environ["ANOTHER_VARIABLE"] = self.testOldEnvVarEndPoint + with mock.patch.object( + Constants, "FOD_CLOUD_API_URL", "ANOTHER_VARIABLE"): + with warnings.catch_warnings(): + warnings.simplefilter("ignore", FutureWarning) + engine = self.new_engine() + self.assertEqual(self.testOldEnvVarEndPoint, engine.baseURL) + + def testEndPointConstant(self): + """! + Check that the constant for the new name holds the new name + """ + + self.assertEqual( + "_51DEGREES_CLOUD_ENDPOINT", Constants.CLOUD_ENDPOINT) + @parameterized.expand([ (False, "query.User-Agent=iPhone", "header.User-Agent=iPhone"), (False, "query.User-Agent=iPhone", "cookie.User-Agent=iPhone"), diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_licensekey_live.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_licensekey_live.py index 6db15de5..aff8943b 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_licensekey_live.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_licensekey_live.py @@ -29,9 +29,12 @@ from fiftyone_pipeline_cloudrequestengine.cloudengine import CloudEngine from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder +from .live_cloud import read_environment + # The licence key the live tests authenticate with, read from the variable # the examples in this repository read it from. -LICENSE_KEY = (os.environ.get("_51DEGREES_LICENSE_KEY") or "").strip() +LICENSE_KEY = read_environment( + "_51DEGREES_LICENSE_KEY", "license_key") or "" USER_AGENT = ( "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) " diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tox.ini b/pipeline/fiftyone_pipeline_cloudrequestengine/tox.ini index 646020a0..6c759a83 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tox.ini +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tox.ini @@ -20,7 +20,9 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_pipeline_cloudrequestengine {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key [testenv:pre-publish] package = skip diff --git a/pipeline/fiftyone_pipeline_core/tox.ini b/pipeline/fiftyone_pipeline_core/tox.ini index 9a03f3a3..1f52d5d0 100644 --- a/pipeline/fiftyone_pipeline_core/tox.ini +++ b/pipeline/fiftyone_pipeline_core/tox.ini @@ -22,7 +22,9 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_pipeline_core {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key [testenv:pre-publish] package = skip diff --git a/pipeline/fiftyone_pipeline_did/examples/creator_context_web/server.py b/pipeline/fiftyone_pipeline_did/examples/creator_context_web/server.py index c3d6dd41..a8b26698 100644 --- a/pipeline/fiftyone_pipeline_did/examples/creator_context_web/server.py +++ b/pipeline/fiftyone_pipeline_did/examples/creator_context_web/server.py @@ -63,10 +63,13 @@ Environment variables. ``_51DEGREES_RESOURCE_KEY`` is required. ``_51DEGREES_LICENSE_KEY`` is optional, and the comment in ``run`` says -why. +why. ``RESOURCE_KEY`` and ``LICENSE_KEY`` are the old names of those two. +They are deprecated, and each is read only when its new name is not set. ``_51DEGREES_CLOUD_ENDPOINT`` is the cloud API base including the ``/api/v4/`` segment, defaulting to ``https://cloud.51degrees.com/api/v4/``, and is the same variable the cloud request engine and the ``DidClient`` honour. +``FOD_CLOUD_API_URL`` is that variable's old name. It is deprecated, and +is read only when ``_51DEGREES_CLOUD_ENDPOINT`` is not set. ``PORT`` is the port to listen on, defaulting to 5100. Standard library plus this package. Run ``python server.py`` then open @@ -79,6 +82,7 @@ import secrets import sys import urllib.parse +import warnings from http.server import BaseHTTPRequestHandler, HTTPServer from pathlib import Path @@ -102,14 +106,27 @@ DEFAULT_API = "https://cloud.51degrees.com/api/v4/" -def env(name): +def env(name, old_name): """The named environment variable when it is set and not empty, - otherwise None.""" - return os.environ.get(name) or None - - -RESOURCE = env("_51DEGREES_RESOURCE_KEY") -LICENCE = env("_51DEGREES_LICENSE_KEY") or "" + otherwise None. When it is unset or empty the name the variable had + before is read, with a warning that the old name is deprecated, as + the ``DidClient`` gives for the cloud endpoint.""" + value = os.environ.get(name) + if value: + return value + value = os.environ.get(old_name) + if not value: + return None + warnings.warn( + "The {0} environment variable is deprecated and {1} replaces it. " + "The value was read from {0} because {1} is not set, so set {1} " + "to the same value.".format(old_name, name), + FutureWarning, stacklevel=2) + return value + + +RESOURCE = env("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY") +LICENCE = env("_51DEGREES_LICENSE_KEY", "LICENSE_KEY") or "" # The cloud API base, normalised to end in exactly one slash so that # every URL is the base followed by its path. The page receives the same # value through its __API__ placeholder and builds its two cloud calls @@ -119,7 +136,8 @@ def env(name): # 51Degrees cloud for performance reasons. That is the private hosting # option of the cloud service, and both run the same service, so this # demo works unchanged against either. -API = (env("_51DEGREES_CLOUD_ENDPOINT") or DEFAULT_API).rstrip("/") + "/" +API = (env("_51DEGREES_CLOUD_ENDPOINT", "FOD_CLOUD_API_URL") + or DEFAULT_API).rstrip("/") + "/" PORT = int(os.environ.get("PORT", "5100")) diff --git a/pipeline/fiftyone_pipeline_did/readme.md b/pipeline/fiftyone_pipeline_did/readme.md index 2d1ce4ad..47ca2fa3 100644 --- a/pipeline/fiftyone_pipeline_did/readme.md +++ b/pipeline/fiftyone_pipeline_did/readme.md @@ -424,7 +424,7 @@ client = DidClient(resource_key, licence_key, transport=httpx_transport) | --- | --- | | `resource_key` | Required. The page's resource key, public by nature. It travels in the route of the key and verify requests and in the form body of the redeem request | | `licence_key` | Optional. A licence key of the same account, server side only. Needed to redeem where the account holds licence keys. Sent only in the body of the redeem request, never in a URL | -| `endpoint` | Optional. The API base including the `/api/v4/` segment. Defaults to the `_51DEGREES_CLOUD_ENDPOINT` environment variable, the same variable the cloud request engine honours, then to `https://cloud.51degrees.com/api/v4/`. A value without a trailing slash gains one | +| `endpoint` | Optional. The API base including the `/api/v4/` segment. Defaults to the `_51DEGREES_CLOUD_ENDPOINT` environment variable, the same variable the cloud request engine honours, then to `FOD_CLOUD_API_URL`, the deprecated old name of that variable, then to `https://cloud.51degrees.com/api/v4/`. The client warns once when the old name is the one used. A value without a trailing slash gains one | | `transport` | Optional. The HTTP transport, either an async callable taking the prepared `urllib.request.Request` and answering `(status, body_bytes)`, or an `urllib.request.OpenerDirector`, whose blocking `open` runs on a worker thread. Defaults to `urllib.request.urlopen` on a worker thread. Tests inject one | | `timeout` | Optional. Seconds the default transport waits for the cloud, 30 by default | | `now` | Optional. The clock, returning an aware UTC `datetime`. Tests inject one | @@ -659,7 +659,7 @@ unreachable cloud answers 502 with `{ "error": ... }`. | --- | --- | | `_51DEGREES_RESOURCE_KEY` | Required. The page's resource key, public by nature | | `_51DEGREES_LICENSE_KEY` | Optional. A licence key of the same account, held server side only. Only an account that holds licence keys needs one to redeem, so an account holding none runs without it | -| `_51DEGREES_CLOUD_ENDPOINT` | Optional. The cloud API base including the `/api/v4/` segment, defaulting to `https://cloud.51degrees.com/api/v4/`. This is the same variable the cloud request engine honours. A host other than cloud.51degrees.com would be used to (a) use an on premise web server, or (b) use a privately hosted version of the 51Degrees cloud for performance reasons, which is the private hosting option of the cloud service. Both run the same service, so the demo works unchanged against either | +| `_51DEGREES_CLOUD_ENDPOINT` | Optional. The cloud API base including the `/api/v4/` segment, defaulting to `https://cloud.51degrees.com/api/v4/`. This is the same variable the cloud request engine honours. `FOD_CLOUD_API_URL` is its old name, which is deprecated and read only when `_51DEGREES_CLOUD_ENDPOINT` is not set. A host other than cloud.51degrees.com would be used to (a) use an on premise web server, or (b) use a privately hosted version of the 51Degrees cloud for performance reasons, which is the private hosting option of the cloud service. Both run the same service, so the demo works unchanged against either | | `PORT` | The port to listen on, defaulting to `5100` | ### Running diff --git a/pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py b/pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py index 72622643..280b770a 100644 --- a/pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py +++ b/pipeline/fiftyone_pipeline_did/src/fiftyone_pipeline_did/did_client.py @@ -53,9 +53,11 @@ import json import os import re +import threading import urllib.error import urllib.parse import urllib.request +import warnings from dataclasses import dataclass from datetime import datetime, timedelta, timezone from enum import Enum @@ -69,12 +71,57 @@ from .id_type import IdType #: The public cloud API base, used when neither the ``endpoint`` argument -#: nor the ``_51DEGREES_CLOUD_ENDPOINT`` environment variable is set. +#: nor the ``_51DEGREES_CLOUD_ENDPOINT`` environment variable, nor that +#: variable's deprecated old name ``FOD_CLOUD_API_URL``, is set. DEFAULT_ENDPOINT = "https://cloud.51degrees.com/api/v4/" #: The environment variable the cloud request engine reads for the API #: base, honoured here when no endpoint argument is given. -ENDPOINT_VARIABLE = "_51DEGREES_CLOUD_ENDPOINT" +CLOUD_ENDPOINT_VARIABLE = "_51DEGREES_CLOUD_ENDPOINT" + +#: Deprecated. Use :data:`CLOUD_ENDPOINT_VARIABLE`. This holds the name that +#: variable had before, which is read when :data:`CLOUD_ENDPOINT_VARIABLE` +#: is not set, so code that passes this constant to the environment reads +#: and sets the old variable. +ENDPOINT_VARIABLE = "FOD_CLOUD_API_URL" + +# Whether the old variable has been reported as deprecated, so that it is +# reported once however many clients are made. +_old_endpoint_reported = False +_old_endpoint_lock = threading.Lock() + + +def _endpoint_from_environment() -> Optional[str]: + """The API base the environment gives, or None when it gives none. + + :data:`CLOUD_ENDPOINT_VARIABLE` is read first. When it is unset or + empty the variable :data:`ENDPOINT_VARIABLE` names is read, which is + the old name, and the first use of it is reported as deprecated. The + report is a :class:`FutureWarning`, which Python shows by default, + because the people who set the variable are the ones running the + application. + """ + global _old_endpoint_reported + endpoint = os.environ.get(CLOUD_ENDPOINT_VARIABLE) + if endpoint: + return endpoint + old_name = ENDPOINT_VARIABLE + endpoint = os.environ.get(old_name) + if not endpoint: + return None + with _old_endpoint_lock: + report = not _old_endpoint_reported + _old_endpoint_reported = True + if report: + # The third frame is the code that made the client. + warnings.warn( + "The {0} environment variable is deprecated and {1} replaces " + "it. The cloud endpoint was read from {0} because {1} is not " + "set, so set {1} to the same value.".format( + old_name, CLOUD_ENDPOINT_VARIABLE), + FutureWarning, stacklevel=3) + return endpoint + _BOUNDARY_TOLERANCE = timedelta(minutes=15) @@ -453,8 +500,10 @@ class DidClient: only. Needed to redeem where the account holds licence keys, and sent only in the body of the redeem request, never in a URL. :param endpoint: the API base including the ``/api/v4/`` segment. - Defaults to the ``_51DEGREES_CLOUD_ENDPOINT`` environment variable, then to - the public cloud. A value without a trailing slash gains one. + Defaults to the ``_51DEGREES_CLOUD_ENDPOINT`` environment variable, + then to that variable's deprecated old name ``FOD_CLOUD_API_URL``, + then to the public cloud. A value without a trailing slash gains + one. :param transport: the HTTP transport, either an async callable taking the prepared :class:`urllib.request.Request` and answering ``(status, body_bytes)``, or an @@ -478,7 +527,7 @@ def __init__(self, resource_key: str, licence_key: Optional[str] = None, raise ValueError("resource_key is required") self._resource_key = resource_key self._licence_key = licence_key if licence_key else None - base = endpoint or os.environ.get(ENDPOINT_VARIABLE) or \ + base = endpoint or _endpoint_from_environment() or \ DEFAULT_ENDPOINT # Normalised to end in exactly one slash so every URL is the base # plus a relative path, as the cloud request engine treats the same diff --git a/pipeline/fiftyone_pipeline_did/tests/live_cloud.py b/pipeline/fiftyone_pipeline_did/tests/live_cloud.py index 23eb9f4f..36bc26ad 100644 --- a/pipeline/fiftyone_pipeline_did/tests/live_cloud.py +++ b/pipeline/fiftyone_pipeline_did/tests/live_cloud.py @@ -22,14 +22,16 @@ """Where the tests that call the live 51Degrees cloud service find their resource key, and what they fail with when there is none or when the -answer lacks what they asked for. +answer lacks what they asked for. Also how a test reads any setting from +an environment variable whose name has changed. -The same file is in each test folder whose tests call the live cloud -service. There is no fallback to a skip, because a cloud test that passes -without calling the cloud has proved nothing. +The same file is in each test folder whose tests read a setting from the +environment. There is no fallback to a skip, because a cloud test that +passes without calling the cloud has proved nothing. """ import os +import sys # Tried in this order. _51DEGREES_RESOURCE_KEY is first because the Cloud # 51Did workflow sets it to each key in turn, and each product's @@ -39,18 +41,68 @@ "_51DEGREES_RESOURCE_KEY_BESPOKE", ) +# Deprecated. The name the resource key variable had before, read when +# none of RESOURCE_KEY_NAMES is set. +OLD_RESOURCE_KEY_NAME = "resource_key" + +# The old names already reported as deprecated, so each is reported once. +_reported_old_names = set() + ACCESSIBLE_PROPERTIES_URL = ( "https://cloud.51degrees.com/api/v4/accessibleproperties?resource=") CREATE_KEY_URL = "https://configure.51degrees.com?utm_source=code&utm_medium=example&utm_campaign=51degrees-python&utm_content=live_cloud.py&utm_term=resource-key-required" +def _report_old_name(name, old_name): + """Prints, once for each old name, that it is deprecated and which + name replaces it.""" + if old_name in _reported_old_names: + return + _reported_old_names.add(old_name) + print( + f"The {old_name} environment variable is deprecated and {name} " + f"replaces it. The value was read from {old_name} because {name} " + f"is not set, so set {name} to the same value.", + file=sys.stderr) + + +def read_environment(name, old_name): + """The value of the environment variable name, or None when it has + none. When name is unset or empty the name the variable had before is + read, and using it prints once that the old name is deprecated.""" + value = os.environ.get(name, "").strip() + if value: + return value + value = os.environ.get(old_name, "").strip() + if not value: + return None + _report_old_name(name, old_name) + return value + + +def is_set(name, old_name): + """Whether the environment variable name is set, whatever it holds, + which is how a switch is read. When it is not set, the name the + variable had before counts, and using it prints once that the old name + is deprecated.""" + if name in os.environ: + return True + if old_name not in os.environ: + return False + _report_old_name(name, old_name) + return True + + def find_resource_key(names=RESOURCE_KEY_NAMES): - """The first resource key set under any of the names, or None.""" + """The first resource key set under any of the names, or None. When + none of RESOURCE_KEY_NAMES is set, the deprecated old name is read.""" for name in names: value = os.environ.get(name, "").strip() if value: return value + if names is RESOURCE_KEY_NAMES: + return read_environment(names[0], OLD_RESOURCE_KEY_NAME) return None diff --git a/pipeline/fiftyone_pipeline_did/tests/test_creator_context_server.py b/pipeline/fiftyone_pipeline_did/tests/test_creator_context_server.py index cd80f8e3..f0784a29 100644 --- a/pipeline/fiftyone_pipeline_did/tests/test_creator_context_server.py +++ b/pipeline/fiftyone_pipeline_did/tests/test_creator_context_server.py @@ -32,9 +32,11 @@ import urllib.error import urllib.parse import urllib.request +import warnings from datetime import timedelta from http.server import HTTPServer from pathlib import Path +from unittest import mock from fiftyone_pipeline_did import DidClient @@ -176,5 +178,125 @@ def test_unreachable_cloud_answers_502_with_an_error(self): self.assertIn("connection refused", json.loads(body)["error"]) +class ServerEndpointVariableTests(unittest.TestCase): + """The demo reads the cloud API base from the environment itself, + because its page needs the same value as its client, so it follows the + same two names as the DidClient. The old name is still read when the + new one is not set.""" + + NEW_VARIABLE = "_51DEGREES_CLOUD_ENDPOINT" + OLD_VARIABLE = "FOD_CLOUD_API_URL" + + def setUp(self): + # Each test starts with neither name set, and what it sets is put + # back afterwards. + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + os.environ.pop(self.NEW_VARIABLE, None) + os.environ.pop(self.OLD_VARIABLE, None) + + def load_recording_warnings(self): + """The server module, loaded afresh so that it reads the + environment again, with the warnings its import gave that say the + old endpoint variable is deprecated and name the new one.""" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + module = load_server() + reports = [ + warning for warning in caught + if issubclass(warning.category, FutureWarning) + and self.OLD_VARIABLE in str(warning.message) + and self.NEW_VARIABLE in str(warning.message)] + return module, reports + + def test_api_defaults_to_the_public_cloud(self): + module, reports = self.load_recording_warnings() + self.assertEqual("https://cloud.51degrees.com/api/v4/", module.API) + self.assertEqual([], reports) + + def test_api_read_from_the_new_variable(self): + os.environ[self.NEW_VARIABLE] = "https://private.example/api/v4" + module, reports = self.load_recording_warnings() + self.assertEqual("https://private.example/api/v4/", module.API) + self.assertEqual([], reports) + + def test_api_read_from_the_old_variable_and_reported(self): + os.environ[self.OLD_VARIABLE] = "https://old.example/api/v4" + module, reports = self.load_recording_warnings() + self.assertEqual("https://old.example/api/v4/", module.API) + self.assertEqual(1, len(reports)) + + def test_new_variable_wins_over_the_old_one(self): + os.environ[self.NEW_VARIABLE] = "https://private.example/api/v4" + os.environ[self.OLD_VARIABLE] = "https://old.example/api/v4" + module, reports = self.load_recording_warnings() + self.assertEqual("https://private.example/api/v4/", module.API) + self.assertEqual([], reports) + + +class ServerKeyVariableTests(unittest.TestCase): + """The demo reads its two keys from the environment, each under its + new name first and then under the name it had before.""" + + NAMES = ( + "_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY", + "_51DEGREES_LICENSE_KEY", "LICENSE_KEY") + + def setUp(self): + # Each test starts with none of the names set, and what it sets is + # put back afterwards. + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + for name in self.NAMES: + os.environ.pop(name, None) + + def load(self): + """The server module, loaded afresh with the environment as it is, + and the warnings that say an old name is deprecated.""" + spec = importlib.util.spec_from_file_location( + "creator_context_server_keys", SERVER_PY) + module = importlib.util.module_from_spec(spec) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + spec.loader.exec_module(module) + reports = [ + str(warning.message) for warning in caught + if issubclass(warning.category, FutureWarning) + and "is deprecated" in str(warning.message)] + return module, reports + + def test_keys_are_read_from_their_old_names_and_reported(self): + os.environ["RESOURCE_KEY"] = "old-resource-key" + os.environ["LICENSE_KEY"] = "old-licence-key" + module, reports = self.load() + self.assertEqual("old-resource-key", module.RESOURCE) + self.assertEqual("old-licence-key", module.LICENCE) + self.assertEqual(2, len(reports)) + self.assertTrue(any( + "RESOURCE_KEY" in report and "_51DEGREES_RESOURCE_KEY" in report + for report in reports)) + self.assertTrue(any( + "LICENSE_KEY" in report and "_51DEGREES_LICENSE_KEY" in report + for report in reports)) + + def test_new_names_win_and_nothing_is_reported(self): + os.environ["_51DEGREES_RESOURCE_KEY"] = "new-resource-key" + os.environ["_51DEGREES_LICENSE_KEY"] = "new-licence-key" + os.environ["RESOURCE_KEY"] = "old-resource-key" + os.environ["LICENSE_KEY"] = "old-licence-key" + module, reports = self.load() + self.assertEqual("new-resource-key", module.RESOURCE) + self.assertEqual("new-licence-key", module.LICENCE) + self.assertEqual([], reports) + + def test_no_keys_gives_none_and_an_empty_licence(self): + module, reports = self.load() + self.assertIsNone(module.RESOURCE) + self.assertEqual("", module.LICENCE) + self.assertEqual([], reports) + + if __name__ == "__main__": unittest.main() diff --git a/pipeline/fiftyone_pipeline_did/tests/test_did_client.py b/pipeline/fiftyone_pipeline_did/tests/test_did_client.py index 3eb64fce..d3a8f4d3 100644 --- a/pipeline/fiftyone_pipeline_did/tests/test_did_client.py +++ b/pipeline/fiftyone_pipeline_did/tests/test_did_client.py @@ -29,6 +29,7 @@ import unittest import urllib.error import urllib.parse +import warnings from datetime import datetime, timedelta, timezone from unittest import mock @@ -50,6 +51,7 @@ SignatureReason, SignatureResult, ) +from fiftyone_pipeline_did import did_client from fiftyone_pipeline_did.did_client import ( KEY_LIST_MAX_AGE, USER_AGENT, parse_iso8601) # The byte layout is not part of the package's public surface. These tests @@ -171,25 +173,85 @@ def test_resource_key_is_required(self): with self.assertRaises(ValueError): DidClient(None) + # The two names of the endpoint environment variable, written out so + # that these tests pin the names themselves. The old one is still read + # when the new one is not set. + NEW_VARIABLE = "_51DEGREES_CLOUD_ENDPOINT" + OLD_VARIABLE = "FOD_CLOUD_API_URL" + + def setUp(self): + # Each test starts with neither name set and with the old name not + # yet reported as deprecated, and what it sets is put back + # afterwards. + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + os.environ.pop(self.NEW_VARIABLE, None) + os.environ.pop(self.OLD_VARIABLE, None) + reported = mock.patch.object( + did_client, "_old_endpoint_reported", False) + reported.start() + self.addCleanup(reported.stop) + + def old_name_reports(self, caught): + """The warnings in a recorded list that say the old endpoint + variable is deprecated and name the one that replaces it.""" + return [ + warning for warning in caught + if issubclass(warning.category, FutureWarning) + and self.OLD_VARIABLE in str(warning.message) + and self.NEW_VARIABLE in str(warning.message)] + def test_endpoint_defaults_to_the_public_cloud(self): - saved = os.environ.pop("_51DEGREES_CLOUD_ENDPOINT", None) - try: - self.assertEqual(DEFAULT_ENDPOINT, DidClient(RESOURCE).endpoint) - finally: - if saved is not None: - os.environ["_51DEGREES_CLOUD_ENDPOINT"] = saved + self.assertEqual(DEFAULT_ENDPOINT, DidClient(RESOURCE).endpoint) def test_endpoint_read_from_the_environment(self): - saved = os.environ.get("_51DEGREES_CLOUD_ENDPOINT") - os.environ["_51DEGREES_CLOUD_ENDPOINT"] = "https://private.example/api/v4" - try: - self.assertEqual("https://private.example/api/v4/", - DidClient(RESOURCE).endpoint) - finally: - if saved is None: - del os.environ["_51DEGREES_CLOUD_ENDPOINT"] - else: - os.environ["_51DEGREES_CLOUD_ENDPOINT"] = saved + os.environ[self.NEW_VARIABLE] = "https://private.example/api/v4" + self.assertEqual("https://private.example/api/v4/", + DidClient(RESOURCE).endpoint) + + def test_endpoint_read_from_the_old_variable(self): + os.environ[self.OLD_VARIABLE] = "https://old.example/api/v4" + with warnings.catch_warnings(): + warnings.simplefilter("ignore", FutureWarning) + client = DidClient(RESOURCE) + self.assertEqual("https://old.example/api/v4/", client.endpoint) + + def test_old_variable_read_when_the_new_one_is_empty(self): + os.environ[self.NEW_VARIABLE] = "" + os.environ[self.OLD_VARIABLE] = "https://old.example/api/v4" + with warnings.catch_warnings(): + warnings.simplefilter("ignore", FutureWarning) + client = DidClient(RESOURCE) + self.assertEqual("https://old.example/api/v4/", client.endpoint) + + def test_empty_old_variable_gives_the_public_cloud(self): + os.environ[self.OLD_VARIABLE] = "" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + client = DidClient(RESOURCE) + self.assertEqual(DEFAULT_ENDPOINT, client.endpoint) + self.assertEqual([], self.old_name_reports(caught)) + + def test_new_variable_wins_over_the_old_one(self): + os.environ[self.NEW_VARIABLE] = "https://private.example/api/v4" + os.environ[self.OLD_VARIABLE] = "https://old.example/api/v4" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + client = DidClient(RESOURCE) + self.assertEqual("https://private.example/api/v4/", client.endpoint) + # The old name was not used, so it is not reported. + self.assertEqual([], self.old_name_reports(caught)) + + def test_old_variable_is_reported_as_deprecated_once(self): + os.environ[self.OLD_VARIABLE] = "https://old.example/api/v4" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + first = DidClient(RESOURCE) + second = DidClient(RESOURCE) + self.assertEqual("https://old.example/api/v4/", first.endpoint) + self.assertEqual("https://old.example/api/v4/", second.endpoint) + self.assertEqual(1, len(self.old_name_reports(caught))) def test_endpoint_argument_wins_and_is_normalised(self): self.assertEqual(ENDPOINT, DidClient( @@ -197,6 +259,47 @@ def test_endpoint_argument_wins_and_is_normalised(self): self.assertEqual(ENDPOINT, DidClient( RESOURCE, endpoint="https://cloud.example/api/v4//").endpoint) + def test_endpoint_argument_wins_over_the_old_variable(self): + os.environ[self.OLD_VARIABLE] = "https://old.example/api/v4" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + client = DidClient(RESOURCE, endpoint=ENDPOINT) + self.assertEqual(ENDPOINT, client.endpoint) + self.assertEqual([], self.old_name_reports(caught)) + + def test_old_constant_keeps_its_old_value(self): + # Code that passes ENDPOINT_VARIABLE to the environment has to keep + # reading the variable it read before. + self.assertEqual(self.OLD_VARIABLE, did_client.ENDPOINT_VARIABLE) + + def test_old_constant_is_given_by_dir_and_a_star_import(self): + # The old constant is a plain attribute of the module, so every + # way of finding it still does. + self.assertIn("ENDPOINT_VARIABLE", dir(did_client)) + self.assertEqual( + self.OLD_VARIABLE, vars(did_client).get("ENDPOINT_VARIABLE")) + names = {} + exec("from fiftyone_pipeline_did.did_client import *", names) + self.assertEqual(self.OLD_VARIABLE, names.get("ENDPOINT_VARIABLE")) + self.assertEqual( + self.NEW_VARIABLE, names.get("CLOUD_ENDPOINT_VARIABLE")) + + def test_client_reads_the_variable_the_old_constant_names(self): + # The client reads the old name through the constant, as it did + # before, so code that points the constant at another variable + # still decides what is read. + os.environ["ANOTHER_VARIABLE"] = "https://other.example/api/v4" + with mock.patch.object( + did_client, "ENDPOINT_VARIABLE", "ANOTHER_VARIABLE"): + with warnings.catch_warnings(): + warnings.simplefilter("ignore", FutureWarning) + client = DidClient(RESOURCE) + self.assertEqual("https://other.example/api/v4/", client.endpoint) + + def test_new_constant_holds_the_new_name(self): + self.assertEqual( + self.NEW_VARIABLE, did_client.CLOUD_ENDPOINT_VARIABLE) + class PublicKeyTests(unittest.TestCase): """Section 2.1: the key list, its cache and the refetch rules.""" diff --git a/pipeline/fiftyone_pipeline_did/tests/test_did_client_live.py b/pipeline/fiftyone_pipeline_did/tests/test_did_client_live.py index fdd0f95c..8ef0fc8a 100644 --- a/pipeline/fiftyone_pipeline_did/tests/test_did_client_live.py +++ b/pipeline/fiftyone_pipeline_did/tests/test_did_client_live.py @@ -51,12 +51,13 @@ ) from fiftyone_pipeline_did.did_client import USER_AGENT -from .live_cloud import answered_without, find_resource_key, no_resource_key +from .live_cloud import ( + answered_without, find_resource_key, no_resource_key, read_environment) pytestmark = pytest.mark.live RESOURCE_KEY = find_resource_key() -LICENCE_KEY = os.environ.get("_51DEGREES_LICENSE_KEY") +LICENCE_KEY = read_environment("_51DEGREES_LICENSE_KEY", "license_key") FODID = "the FODid product" MARKETING = "the CloudV5FODiD product" diff --git a/pipeline/fiftyone_pipeline_did/tests/test_live_cloud.py b/pipeline/fiftyone_pipeline_did/tests/test_live_cloud.py new file mode 100644 index 00000000..415e24ec --- /dev/null +++ b/pipeline/fiftyone_pipeline_did/tests/test_live_cloud.py @@ -0,0 +1,184 @@ +# ********************************************************************* +# This Original Work is copyright of 51 Degrees Mobile Experts Limited. +# Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, +# Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. +# +# This Original Work is licensed under the European Union Public Licence +# (EUPL) v.1.2 and is subject to its terms as set out below. +# +# If a copy of the EUPL was not distributed with this file, You can obtain +# one at https://opensource.org/licenses/EUPL-1.2. +# +# The 'Compatible Licences' set out in the Appendix to the EUPL (as may be +# amended by the European Commission) shall be deemed incompatible for +# the purposes of the Work and the provisions of the compatibility +# clause in Article 5 of the EUPL shall not apply. +# +# If using the Work as, or as part of, a network application, by +# including the attribution notice(s) required under Article 5 of the EUPL +# in the end user terms of the application under an appropriate heading, +# such notice(s) shall fulfill the requirements of that article. +# ********************************************************************* + +"""Checks live_cloud.py, which every test folder that reads a setting from +the environment holds a copy of. A variable whose name changed is still +read under its old name when the new one is not set, and using the old name +prints once that it is deprecated. These need no resource key. +""" + +import contextlib +import io +import os +import unittest +from pathlib import Path +from unittest import mock + +from . import live_cloud + +# Every folder that holds a copy of live_cloud.py, from the root of the +# repository. +COPIES = ( + "device-detection/fiftyone_devicedetection_cloud/tests", + "device-detection/fiftyone_devicedetection_examples/tests", + "ip-intelligence/fiftyone_ipintelligence_examples/tests", + "ip-intelligence/fiftyone_ipintelligence_onpremise/tests", + "location/tests", + "pipeline/fiftyone_pipeline_cloudrequestengine/examples/tests", + "pipeline/fiftyone_pipeline_cloudrequestengine/tests", + "pipeline/fiftyone_pipeline_did/tests", +) + +NAMES = ( + "_51DEGREES_RESOURCE_KEY", "_51DEGREES_RESOURCE_KEY_BESPOKE", + "_51DEGREES_RESOURCE_KEY_BASIC", "resource_key", + "_51DEGREES_LICENSE_KEY", "license_key") + + +class OldNameTests(unittest.TestCase): + + def setUp(self): + # Each test starts with none of the names set and no old name + # reported yet, and what it sets is put back afterwards. + environment = mock.patch.dict(os.environ) + environment.start() + self.addCleanup(environment.stop) + for name in NAMES: + os.environ.pop(name, None) + reported = mock.patch.object( + live_cloud, "_reported_old_names", set()) + reported.start() + self.addCleanup(reported.stop) + + def printed(self, call): + """What a call returned and what it printed to the error stream.""" + errors = io.StringIO() + with contextlib.redirect_stderr(errors): + value = call() + return value, errors.getvalue() + + def test_resource_key_is_read_from_the_old_name(self): + os.environ["resource_key"] = "old-key" + value, errors = self.printed(live_cloud.find_resource_key) + self.assertEqual("old-key", value) + self.assertIn("resource_key environment variable is deprecated", + errors) + self.assertIn("_51DEGREES_RESOURCE_KEY", errors) + + def test_new_resource_key_name_wins_and_nothing_is_printed(self): + os.environ["_51DEGREES_RESOURCE_KEY"] = "new-key" + os.environ["resource_key"] = "old-key" + value, errors = self.printed(live_cloud.find_resource_key) + self.assertEqual("new-key", value) + self.assertEqual("", errors) + + def test_no_resource_key_gives_none(self): + value, errors = self.printed(live_cloud.find_resource_key) + self.assertIsNone(value) + self.assertEqual("", errors) + + def test_other_names_do_not_fall_back_to_the_old_name(self): + # The old name was one key, so a search for a key of another tier + # does not take it. + os.environ["resource_key"] = "old-key" + value, _ = self.printed(lambda: live_cloud.find_resource_key( + ("_51DEGREES_RESOURCE_KEY_BASIC",))) + self.assertIsNone(value) + + def test_old_name_is_read_and_reported_once(self): + os.environ["license_key"] = "old-licence" + + def twice(): + return ( + live_cloud.read_environment( + "_51DEGREES_LICENSE_KEY", "license_key"), + live_cloud.read_environment( + "_51DEGREES_LICENSE_KEY", "license_key")) + + value, errors = self.printed(twice) + self.assertEqual(("old-licence", "old-licence"), value) + self.assertEqual( + 1, errors.count("license_key environment variable is deprecated")) + self.assertIn("_51DEGREES_LICENSE_KEY", errors) + + def test_old_name_is_read_when_the_new_one_is_empty(self): + os.environ["_51DEGREES_LICENSE_KEY"] = "" + os.environ["license_key"] = "old-licence" + value, _ = self.printed(lambda: live_cloud.read_environment( + "_51DEGREES_LICENSE_KEY", "license_key")) + self.assertEqual("old-licence", value) + + def test_new_name_wins_and_nothing_is_printed(self): + os.environ["_51DEGREES_LICENSE_KEY"] = "new-licence" + os.environ["license_key"] = "old-licence" + value, errors = self.printed(lambda: live_cloud.read_environment( + "_51DEGREES_LICENSE_KEY", "license_key")) + self.assertEqual("new-licence", value) + self.assertEqual("", errors) + + def test_neither_name_gives_none(self): + value, errors = self.printed(lambda: live_cloud.read_environment( + "_51DEGREES_LICENSE_KEY", "license_key")) + self.assertIsNone(value) + self.assertEqual("", errors) + + def test_old_name_set_to_nothing_still_counts_as_set(self): + # A switch is on when its variable is set, whatever it holds. + os.environ["license_key"] = "" + value, errors = self.printed(lambda: live_cloud.is_set( + "_51DEGREES_LICENSE_KEY", "license_key")) + self.assertTrue(value) + self.assertEqual( + 1, errors.count("license_key environment variable is deprecated")) + self.assertIn("_51DEGREES_LICENSE_KEY", errors) + + def test_new_name_set_to_nothing_counts_as_set(self): + os.environ["_51DEGREES_LICENSE_KEY"] = "" + value, errors = self.printed(lambda: live_cloud.is_set( + "_51DEGREES_LICENSE_KEY", "license_key")) + self.assertTrue(value) + self.assertEqual("", errors) + + def test_neither_name_is_not_set(self): + value, errors = self.printed(lambda: live_cloud.is_set( + "_51DEGREES_LICENSE_KEY", "license_key")) + self.assertFalse(value) + self.assertEqual("", errors) + + +class CopiesTests(unittest.TestCase): + + def test_every_copy_is_the_same_file(self): + # One copy is tested here, so the others have to be that file. + root = Path(__file__).resolve().parents[3] + if not (root / "ci" / "packages.json").is_file(): + self.skipTest("not run from a checkout of the repository") + expected = Path(__file__).with_name("live_cloud.py").read_bytes() + for folder in COPIES: + with self.subTest(folder=folder): + copy = root / folder / "live_cloud.py" + self.assertTrue(copy.is_file(), str(copy)) + self.assertEqual(expected, copy.read_bytes()) + + +if __name__ == "__main__": + unittest.main() diff --git a/pipeline/fiftyone_pipeline_did/tox.ini b/pipeline/fiftyone_pipeline_did/tox.ini index 7317c461..f42e48fb 100644 --- a/pipeline/fiftyone_pipeline_did/tox.ini +++ b/pipeline/fiftyone_pipeline_did/tox.ini @@ -16,10 +16,13 @@ wheel_build_env = .pkg # scripts select with. passenv = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_RESOURCE_KEY_BESPOKE _51DEGREES_RESOURCE_KEY_BASIC _51DEGREES_LICENSE_KEY + license_key _51DEGREES_CLOUD_ENDPOINT + FOD_CLOUD_API_URL PYTEST_ADDOPTS deps = {[common]deps} diff --git a/pipeline/fiftyone_pipeline_engines/tox.ini b/pipeline/fiftyone_pipeline_engines/tox.ini index 1e8e9619..7ca15783 100644 --- a/pipeline/fiftyone_pipeline_engines/tox.ini +++ b/pipeline/fiftyone_pipeline_engines/tox.ini @@ -19,7 +19,9 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_pipeline_engines {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key [testenv:pre-publish] package = skip diff --git a/pipeline/fiftyone_pipeline_engines_fiftyone/tox.ini b/pipeline/fiftyone_pipeline_engines_fiftyone/tox.ini index cddb3cd4..5e9b48a8 100644 --- a/pipeline/fiftyone_pipeline_engines_fiftyone/tox.ini +++ b/pipeline/fiftyone_pipeline_engines_fiftyone/tox.ini @@ -20,7 +20,9 @@ commands = pytest --cov={envsitepackagesdir}/fiftyone_pipeline_engines_fiftyone {tty:--color=yes} {posargs} pass_env = _51DEGREES_RESOURCE_KEY + resource_key _51DEGREES_LICENSE_KEY + license_key [testenv:pre-publish] package = skip