Repository navigation
Keep reading the old cloud endpoint name and bring back its constants - #116
Merged
Merged
Conversation
A patch release keeps every public name, and the name of an environment variable a package reads is a public name. These tests ask for FOD_CLOUD_API_URL to be read when _51DEGREES_CLOUD_ENDPOINT is not set, for _51DEGREES_CLOUD_ENDPOINT to win when both are set, for the use of the old name to be logged as deprecated once, and for the two constants that held the old name to hold it again. They cover the cloud request engine builder, the 51Did client, which is run in a Java process of its own so that it reads a real environment, and the cloud engine example. The cases of the environment variable name check now expect the old endpoint name to be allowed. The tests fail until the fix that follows.
The cloud endpoint variable was renamed from FOD_CLOUD_API_URL to _51DEGREES_CLOUD_ENDPOINT, and the old name was no longer read. A patch release keeps every public name, and that covers the name of an environment variable a package reads. The cloud request engine builder, the 51Did client and the cloud engine example now read _51DEGREES_CLOUD_ENDPOINT first and FOD_CLOUD_API_URL when that is not set or is empty. The first use of the old name in a process is logged as a warning that names its replacement. The engine builder logs through its logger factory, the example through its own, and the 51Did client through java.util.logging, because that module has no logging dependency and this adds none. Constants.FOD_CLOUD_API_URL comes back holding "FOD_CLOUD_API_URL", deprecated and pointing at Constants.CLOUD_ENDPOINT_VARIABLE. DidClient.ENDPOINT_VARIABLE holds "FOD_CLOUD_API_URL" again and is deprecated, and the new DidClient.CLOUD_ENDPOINT_VARIABLE holds "_51DEGREES_CLOUD_ENDPOINT". The environment variable name check no longer fails on FOD_CLOUD_API_URL, because the packages read it again.
Every environment variable name that a package, a test, an example or a CI script reads is a public name, so a patch release keeps reading the old one when the new one is not set. These tests ask for that wherever main read a name that rc had stopped reading. 51DEGREES_IPI_PATH and 51DEGREES_RESOURCE_KEY in the IP Intelligence test helpers and example helpers, 51DEGREES_DD_PATH in the on-premise web example, and RESOURCE_KEY and LICENSE_KEY in the cloud engine example, the 51Did example and the 51Did live test keys. They also ask for the four helper constants that held an old name to hold it again. The cases of the environment variable name check now expect an old name to be allowed only in the files that read it as a deprecated fallback, and refused everywhere else. The tests fail until the fix that follows.
…back Every environment variable name that a package, a test, an example or a CI script reads is a public name. Each reader that main had and rc changed now reads the new name first and the old name when the new one is not set or is empty, and says once that the old name is deprecated and which name replaces it. 51DEGREES_IPI_PATH is read again by the IP Intelligence test helper FileUtils, the example helper DataFileHelper and the country translation integration test. 51DEGREES_RESOURCE_KEY is read again by the test helper KeyUtils and the example helper KeyHelper. 51DEGREES_DD_PATH is read again by the on-premise web example's configuration. RESOURCE_KEY is read again by the cloud engine example, the 51Did example and the 51Did live test keys, and LICENSE_KEY by the last two. FileUtils.IPI_PATH_ENV_VAR, KeyUtils.RESOURCE_KEY_ENV_VAR, DataFileHelper.IPI_PATH_ENV_VAR and KeyHelper.RESOURCE_KEY_ENV_VAR hold their old values again and are deprecated. IPI_PATH_VARIABLE and RESOURCE_KEY_VARIABLE beside them hold the new names, and the code that named the old constants now names those. The environment variable name check has FOD_CLOUD_API_URL back in its pattern, and allows each old name only in the files that read it as a deprecated fallback or test that read.
Two tests run a helper with nothing set, and each checked and cleared only the new name of a variable. The helpers now read the old name when the new one is not set, so each test failed wherever the old name was set. KeyHelperTest.getOrSetTestResourceKey_NoKey_StoresNothing now also checks and clears 51DEGREES_RESOURCE_KEY, and ExamplesWithoutSettingsTest also checks and clears 51DEGREES_IPI_PATH, through the deprecated constant that holds it.
jwrosewell
marked this pull request as ready for review
November 12, 2026 21:01
This was referenced Nov 12, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
rcrenamed environment variables and stopped reading the old names, in what is a patch release. #105, "Give every environment variable one 51DEGREES name", renamed the cloud endpoint variable fromFOD_CLOUD_API_URLto_51DEGREES_CLOUD_ENDPOINTand gave the data file and key variables names that start with an underscore. Every file in the first table below is among the files that pull request changed. A deployment, a developer or a build that sets an old name would, after upgrading, be ignored with no error and no message. For the endpoint that means requests meant for a private host go tocloud.51degrees.com.Six public constants that held an old name were also renamed or given a new value, so code that names
Constants.FOD_CLOUD_API_URLno longer compiles, and code that passes one of the other five to the environment reads a different variable.#105 was opened and merged into
rcbyjwrosewellat 19:36 UTC on 11 November 2026, with no review recorded on the pull request. Nothing has been released with the renames, becausemainat f69da52 still holds the old names. The release draft #112 waits for this change.The rules this follows
James Rosewell, at about 11:40 UTC on 12 November 2026:
Asked at about 12:05 UTC whether that covers an environment variable name a package reads as well as names in code, he answered:
Asked at about 16:55 UTC whether it covers environment variable names that tests and examples read as well as packages, he answered:
So every environment variable name that a package, a test, an example or a CI script in this repository reads is a public name. Where a name changed, the old name is read when the new one is not set or is empty, with a message that the old name is deprecated and which name replaces it. A package logs the message once through its own logger, and a test helper or an example prints or logs it once. This draft began with the cloud endpoint alone and was widened to every name after the third answer, in the same draft.
No specification names these variables. A code search of the default branch of the specifications repository on 12 November 2026 finds neither
FOD_CLOUD_API_URLnor_51DEGREES_CLOUD_ENDPOINT.Every name that changed, and where it is read
The list comes from every file of
mainat f69da52 and ofrcat 0b9257f, searched for each old and new name and for everySystem.getenvcall. The last release of every package, 4.5.130 of 5 November 2026, is tagged at that samemaincommit. Each row is one reader. The last column is at the head, 02b1bca.mainread the old namercread only the new nameFOD_CLOUD_API_URL_51DEGREES_CLOUD_ENDPOINTendPointFromEnvironment, lines 165 to 190FOD_CLOUD_API_URL_51DEGREES_CLOUD_ENDPOINTresolveEndpoint, lines 1174 to 1206FOD_CLOUD_API_URL_51DEGREES_CLOUD_ENDPOINTendpoint, lines 92 to 119RESOURCE_KEY_51DEGREES_RESOURCE_KEYconfiguredResourceKey, lines 141 to 162RESOURCE_KEY,LICENSE_KEY_51DEGREES_RESOURCE_KEY,_51DEGREES_LICENSE_KEYkey, lines 472 to 486RESOURCE_KEY,LICENSE_KEY_51DEGREES_RESOURCE_KEY,_51DEGREES_LICENSE_KEYLiveCloudKeys.find, lines 90 to 11251DEGREES_IPI_PATH_51DEGREES_IPI_PATHFileUtilsgetExplicitHashFile, lines 123 to 14451DEGREES_IPI_PATH_51DEGREES_IPI_PATHDataFileHelpergetDataFileLocation, lines 431 to 45851DEGREES_IPI_PATH_51DEGREES_IPI_PATHpathFromEnvironment, lines 77 to 9351DEGREES_RESOURCE_KEY_51DEGREES_RESOURCE_KEYKeyUtilsgetResourceKey, lines 63 to 8351DEGREES_RESOURCE_KEY_51DEGREES_RESOURCE_KEYKeyHelperrequireResourceKey,oldNameResourceKeyandgetOrSetResourceKey, lines 84 to 20951DEGREES_DD_PATH_51DEGREES_DD_PATHwarnOfOldDataFileName, lines 73 to 82Names that did not change between
mainandrcneed nothing, beingTestResourceKey,SuperResourceKey,TestDataFile,LicenseKey,TestCloudEndPointand the_51DEGREES_RESOURCE_KEY_BESPOKE,_51DEGREES_RESOURCE_KEY_51DIDand_51DEGREES_LICENSE_KEY_51DIDnames the workflows set. No CI script onmainread or set an old name.Three of the old names start with a digit. On
maineach of their readers took the name as an environment variable and then as a Java system property, and the fallback keeps both. On the Ubuntu runner only the system property reached the tests. Run 43287526584 set two of those names in the environment of the step that runs Maven, and the two tests that fail when the names are read passed. Run 43289150046 gave the same two names as system properties and those tests failed. That second run also printed what a child process saw of the names in the environment, which was both names for a child ofbashand neither for a child ofsh.The constants that held an old name:
mainrcConstants.FOD_CLOUD_API_URL(cloud request engine, package)"FOD_CLOUD_API_URL"CLOUD_ENDPOINT_VARIABLE"FOD_CLOUD_API_URL",@DeprecatedConstants.CLOUD_ENDPOINT_VARIABLEDidClient.ENDPOINT_VARIABLE(51Did, package)"FOD_CLOUD_API_URL""_51DEGREES_CLOUD_ENDPOINT""FOD_CLOUD_API_URL",@DeprecatedDidClient.CLOUD_ENDPOINT_VARIABLE, newFileUtils.IPI_PATH_ENV_VAR(IP Intelligence test helpers, package)"51DEGREES_IPI_PATH""_51DEGREES_IPI_PATH""51DEGREES_IPI_PATH",@DeprecatedFileUtils.IPI_PATH_VARIABLE, newKeyUtils.RESOURCE_KEY_ENV_VAR(IP Intelligence test helpers, package)"51DEGREES_RESOURCE_KEY""_51DEGREES_RESOURCE_KEY""51DEGREES_RESOURCE_KEY",@DeprecatedKeyUtils.RESOURCE_KEY_VARIABLE, newDataFileHelper.IPI_PATH_ENV_VAR(IP Intelligence examples)"51DEGREES_IPI_PATH""_51DEGREES_IPI_PATH""51DEGREES_IPI_PATH",@DeprecatedDataFileHelper.IPI_PATH_VARIABLE, newKeyHelper.RESOURCE_KEY_ENV_VAR(IP Intelligence examples)"51DEGREES_RESOURCE_KEY""_51DEGREES_RESOURCE_KEY""51DEGREES_RESOURCE_KEY",@DeprecatedKeyHelper.RESOURCE_KEY_VARIABLE, newWhat changed
java.util.logging, because that module depends only onorg.jsonand its build file says it adds no dependency the pipeline does not carry). The examples log a warning through the SLF4J logger they already hold, except the 51Did example, which has no logger and prints to standard error, as the test helpers do. Every message has the same form, for example "The environment variable FOD_CLOUD_API_URL is deprecated and _51DEGREES_CLOUD_ENDPOINT replaces it. Set _51DEGREES_CLOUD_ENDPOINT to the same value, as a later release may no longer read FOD_CLOUD_API_URL."@Deprecated, with a@deprecatednote pointing at the constant that holds the new name. Code in this repository that named one of the old constants to tell a reader which variable to set now names the new constant, so every message and document still gives the_51DEGREES_name.DataFilesetting, and the warning comes from the example'smain.ci/check-environment-variable-names.ps1keepsFOD_CLOUD_API_URL,RESOURCE_KEYandLICENSE_KEYin its pattern and still fails on any name that starts with a digit.DeprecatedFallbacks, lines 77 to 117 lists the 22 files that read an old name or test that read, each with the old names it may hold, andTest-DeprecatedFallback, lines 123 to 130 lets a match through only for a listed file and a name listed for it, in its exact case. Everywhere else the check fails as before.pipeline/README.md, line 142 says the old endpoint name is deprecated and still read.One name is left as
rchas it. The 51Did workflow onmainpassed the organisation variable_51DEGREES_RESOURCE_KEY_BASICto its tests, and #59 took it out, saying the workflow "drops the basic key, which does not carry the fodid product: the tests now insist on a 51Did, so that key would report a failure that is entitlement working correctly". That is a key the workflow no longer runs with and not a name that changed, and putting it back would turn the check red, so this draft does not touch it.Tests
The tests went in first each time. Commit 2e7bdc6e is the endpoint tests alone and 98996a06 the endpoint fix. Commit a5b1ee6f is the tests for every other name alone and 3b2bef1d the fix for them. Commit 02b1bca0 changes two existing tests that review found failing wherever an old name is set, and nothing else.
CloudRequestEngineBuilderTests, lines 194 to 268, six newConstants.FOD_CLOUD_API_URLholds"FOD_CLOUD_API_URL"and carries@Deprecated. The existingCloudEndPoint_Default_Settingcovers neither name setDidClientEnvironmentTests, six new, withEndpointProbeSystem.getenvdirectly and a Java process cannot change its own environment, so each test starts a second Java process with the variables it needs. Only the old name gives its endpoint, and logs one warning for two clients. A blank new name gives the old name's endpoint. Both set gives the new name's. Neither gives the default.DidClient.ENDPOINT_VARIABLEholds"FOD_CLOUD_API_URL"and carries@DeprecatedLiveCloudKeysTests, four newRESOURCE_KEYandLICENSE_KEYare each read when their new name is not set, the new name wins, and an old name is read only in place of its own new nameEndpointTestsof the cloud engine example, five newRESOURCE_KEY, and both key namesDeprecatedKeyNameTestsof the 51Did example, four new, withKeyProbeFileUtilsTestandKeyUtilsTest, three new each@Deprecatedand has a new constant beside itDataFileHelperTestandKeyHelperTest, three new eachOnPremDataFileOptionTest, two new51DEGREES_DD_PATHahead ofTestDataFile, and from_51DEGREES_DD_PATHahead of bothcheck-environment-variable-names.Tests.ps1, lines 93 to 150, 15 new casesDidClient.javaholds the old endpoint name and fails one that holds it in any other file.Test-DeprecatedFallbackallows five name and file pairs and refuses eight, among them a listed file with a name it is not listed for and a name in the wrong caseKeyHelperTest.getOrSetTestResourceKey_NoKey_StoresNothing, lines 52 to 85 andExamplesWithoutSettingsTest.runWithoutSettings, lines 57 to 96, existing testsThe country translation integration test has no test of its own for the old name, because it reads the environment directly, needs a data file and is skipped without one.
Runs
oldNameAlone_GivesItsEndpointwith "expected:<[https://old.example/api/v4/, ...]> but was:<[https://cloud.51degrees.com/api/v4/, ...]>". The other eight configurations were cancelled when that one failed, and Maven stopped at that moduleKeyUtilsTest.oldKeyNameIsReadWhenNewIsNotSetwith "expected: but was:" andOnPremDataFileOptionTest.usesOldDdPathWhenNewIsNotSetwith "expected:<from-[old-dd-path].hash> but was:<from-[test-data-file].hash>". The two that passed are the endpoint classes, which 98996a0 had already fixed51DEGREES_RESOURCE_KEYset to a value shaped like a key and51DEGREES_IPI_PATHset to the rootpom.xml, both as system properties.getOrSetTestResourceKey_NoKey_StoresNothingfailed with "expected null, but was:", and 8 of the 9ExamplesWithoutSettingsTesttests ended in an error atrunWithoutSettings, because the examples took the pom as their data fileSetting 'PullRequestSha'line in Configure and in each of the nine build jobs is 02b1bca, and Configure printed "Left out 6 macOS entries". In each of the nine, the eleven JUnit test classes listed under Tests ran their 63 tests with none failed and none skippedmacOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.
What remains
DeprecatedFallbacksinci/check-environment-variable-names.ps1.Constants.FOD_CLOUD_API_URL"is now"Constants.CLOUD_ENDPOINT_VARIABLE, and its "Environment variables" section says the examples, tests and CI scripts "read only the new names". Once this merges, both need to say that every old name is deprecated and still read.tmp/endpoint-java-red,tmp/endpoint-java-green,tmp/endpoint-java-red2,tmp/endpoint-java-green2,tmp/endpoint-java-red4andtmp/endpoint-java-green4are kept so the commits linked above stay reachable. They can be deleted once this is merged.DeprecatedFallbacksin the name check and the README paragraph should refer to it.