Repository navigation
Read the cloud endpoint from _51DEGREES_CLOUD_ENDPOINT - #77
Merged
Merged
Conversation
The cloud request engine and the 51Did client must read their endpoint from _51DEGREES_CLOUD_ENDPOINT and not from the old name, and the environment variable name check now fails on any file that still names the old one.
The cloud request engine and the 51Did client read their endpoint from _51DEGREES_CLOUD_ENDPOINT, the name the other 51Degrees languages use, with no fallback to the old name. The examples, readmes, type declarations, integration tests and the CI script name the new variable.
Contributor
Author
|
Closing and reopening at once to start the checks again, because the runs queued during GitHub's Actions outage this evening never started. |
jwrosewell
marked this pull request as ready for review
November 11, 2026 23:51
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.
The cloud request engine and the 51Did client read their endpoint from
_51DEGREES_CLOUD_ENDPOINTin place ofFOD_CLOUD_API_URL, with no fallback, for consistency with the other languages in the 11 November release. The environment variable name check now rejects the old name anywhere in the repository.The problem
Node read the endpoint from
FOD_CLOUD_API_URLin two places, the cloud request engine (cloudRequestEngine.js line 143 at 8093efa) and the 51Did client (didClient.js line 1009). The CI script set it for the cloud contract tests (ci/run-integration-tests.ps1 line 88), and four IP Intelligence examples, the creator context web example, two readmes, the 51Did type declarations and two integration tests named it.Rust reads
_51DEGREES_CLOUD_ENDPOINT(README.md line 419 at 245bc39), and the drafts 51degrees-dotnet#131, 51degrees-java#105, 51degrees-python#100, 51degrees-php#68 and 51degrees-go#102 make the same rename. .NET's draft takesFOD_CLOUD_API_URLandFIFTYONE_CLOUD_ENDPOINTto_51DEGREES_CLOUD_ENDPOINTunder the decision it quotes, "Just use the version that works everywhere all the time with no fallback."What changed
cloudRequestEngine.jsanddidClient.jsreadprocess.env._51DEGREES_CLOUD_ENDPOINT. Nothing reads the old name. ThebaseURLandendpointoptions still win over the variable, and the public cloud is still the default.types/didClient.d.ts, the two 51Did integration tests andci/run-integration-tests.ps1name the new variable. Two comments, one readme paragraph and one error message that the longer name pushed past 80 columns are rewrapped.ci/tests/environmentVariableNames.test.jsgainsOLD_NAME, which matches the whole old name in any letter case wherever it appears, read, set or in prose, and leaves a longer name holding it alone. The test "names no old cloud endpoint variable" runs it over every tracked file, and "matches only the whole old cloud endpoint name" checks what it matches.Tests, red then green
New and changed tests:
cloudRequestEngine.test.jssets_51DEGREES_CLOUD_ENDPOINTin the two endpoint precedence tests, and the new "custom end point - old variable name not read" sets only the old name and expects the default.didClient.test.jsuses the new name throughout, and the new "endpoint does not read the old variable name" does the same for the 51Did client.The tests in
cloudRequestEngine.test.jsanddidClient.test.jsthat name the old name build it in two parts,'FOD' + '_CLOUD_API_URL', so the guard does not find it in them. The guard file itself does not do that.ci/tests/environmentVariableNames.test.jscarries the old name as a literal nine times, once inOLD_NAMEand eight times in its own test of whatOLD_NAMEmatches, and the scan passes over it only because that file is the one entry inSKIPPED_FILES(lines 63 to 66 at 80a1bf7), whichnamesInRepositoryleaves out of every scan (line 120). So a name added to the guard's own tests is not found by the guard, and a name added anywhere else is.The red and green runs used a temporary workflow on throwaway branches, since deleted, which ran these three test files with Jest.
Full build
The head is 80a1bf7, which merges
rcat f76d8b0 into 0948eb3.rchad taken #45 since this draft was opened, which also changescloudRequestEngine.jsandcloudRequestEngine.test.js, and the merge was clean. The red and green above are of 0948eb3, and the merge changes no line this draft changes.shareUsageBackOff.test.js"share usage back-off - full queue drops new evidence", which sent four of the six entries it expects to be dropped. This draft changes no usage sharing file, and the same test passed on both Ubuntu ARM jobs of #75's full build on the samerc. The test expects ten processes to finish inside a two second pause, so a slow runner can end the pause early. The test needs a change of its own, which this draft does not make.Setting 'PullRequestSha'lines all name 80a1bf7, Configure printed "Left out 2 macOS entries", and Build and Test passed with Node 22 and 24 on Ubuntu, Ubuntu ARM and Windows. On each of the six, the unit tests gave 1,225 passed and 2 skipped, the integration tests, whichci/run-integration-tests.ps1runs with the renamed variable, gave 132 passed and 4 skipped, andenvironmentVariableNames.test.js,cloudRequestEngine.test.jsanddidClient.test.jsall passed.macOS 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.
Release note
_51DEGREES_CLOUD_ENDPOINTin place ofFOD_CLOUD_API_URL. The old name is no longer read, so set the new one.What remains
Nothing in this repository. The same rename in the other languages is in the drafts listed above.