node-server-sdk icon indicating copy to clipboard operation
node-server-sdk copied to clipboard

Unrestrict poll interval when not talking to official LD server

Open declanshanaghy opened this issue 2 years ago • 6 comments

Requirements

  • ❌ I have not added test coverage for new or changed functionality because there were no test on this code path to update
  • ✅ I have followed the repository's pull request submission guidelines
  • ✅ I have validated my changes against all supported platform versions

Describe the solution you've provided

In test environments where LaunchDarkly is replaced with a stub server it's desirable to change flags to test both variations. Most stub servers don't support SSE modes so short interval polling is necessary.

This PR allows setting the minimum below 30 seconds if not talking to the official LD API server

Describe alternatives you've considered

Attempted to find a test stub server that supports SSE but none exist.

declanshanaghy avatar Mar 23 '22 16:03 declanshanaghy

@declanshanaghy Would either TestData, or FileData be applicable for your use case? When using a file data source, or a test data source, the client will not connect to a server, and instead can use flags from memory (TestData), or flags from a file (FileData). The flags can then be toggled in memory, or from a file on disk, instead of a server.

Using test data:

     const { TestData } = require('launchdarkly-node-server-sdk/interfaces');
 
     const td = TestData();
     testData.update(td.flag("flag-key-1").booleanFlag().variationForAllUsers(true));
     const client = new LDClient(sdkKey, { updateProcessor: td });

     // flags can be updated at any time:
     td.update(td.flag("flag-key-2")
         .variationForUser("some-user-key", true)
         .fallthroughVariation(false));

Using file data:

     const { FileDataSource } = require('launchdarkly-node-server-sdk/integrations');
 
     const dataSource = FileDataSource({ paths: [ myFilePath ], autoUpdate: true }); // The autoUpdate here will cause the file to be watched. 
     const config = { updateProcessor: dataSource };

https://docs.launchdarkly.com/sdk/features/flags-from-files#nodejs-server-side

kinyoklion avatar Mar 23 '22 17:03 kinyoklion

Given the test in question have a few quidelines in place that make this preferable

  • mimic the real environment as much as possible.
  • maintain consistent patterns across dependencies for stubbing (i.e: replace with HTTP stubs)

In the case of using a file we would need to mount a shared volume hosting that file between the containers hosting our server and the test process.

declanshanaghy avatar Mar 23 '22 18:03 declanshanaghy

@declanshanaghy I see why you do not want to use test data or the file data given the constraints of your environment. That said, I have concerns with the approach of this PR. The URL being different than the default does not indicate that the URL is not production. There are different production configurations of the SDK where the URL will not be the default URL.

kinyoklion avatar Mar 28 '22 16:03 kinyoklion

Is there another mechanism that could be used to determine when it's ok to lower the threshold?

I'm not familiar enough with all the use cases to see an alternative myself at this time.

declanshanaghy avatar Mar 29 '22 18:03 declanshanaghy

@declanshanaghy I was thinking about it some, and I have not thought of a safe/reasonable alternative as of yet. Would you mind pointing me at one of the stubbing servers that you are using?

kinyoklion avatar Mar 29 '22 19:03 kinyoklion

@kinyoklion This fell off my radar. We're using WireMock in docker-compose

declanshanaghy avatar May 18 '23 21:05 declanshanaghy