RediStack
RediStack copied to clipboard
swift-server
Make connection retry timeout default consistent
Fix for #104. Makes the connection retry timeout consistent regardless of whether no value is specified, or nil is specified.
Test failures seem to be due to the specific test comparing floats:
Test Suite 'StringCommandsTests' failed at 2024-03-20 15:05:53.225. Executed 17 tests, with 2 failures (0 unexpected) in 0.576 (0.577) seconds Test Suite 'RediStackPackageTests.xctest' failed at 2024-03-20 15:05:53.225. Executed 264 tests, with 2 failures (0 unexpected) in 38.703 (38.716) seconds Test Suite 'All tests' failed at 2024-03-20 15:05:53.225. Executed 264 tests, with 2 failures (0 unexpected) in 38.703 (38.717) seconds
…/RediStack/Tests/RediStackIntegrationTests/Commands/StringCommandsTests.swift:212: error: -[RediStackIntegrationTests.StringCommandsTests test_incrementByFloat] : XCTAssertEqual failed: ("3.1479989999999987") is not equal to ("3.147999")
…/RediStack/Tests/RediStackIntegrationTests/Commands/StringCommandsTests.swift:214: error: -[RediStackIntegrationTests.StringCommandsTests test_incrementByFloat] : XCTAssertEqual failed: ("18.441798999999996") is not equal to ("18.441799")
I'm not sure what the tests are trying to do here, but I notice that only "soundness," 5.6, 5.7, and 5.8 have a "required" label, but the rest do not (in particular, 5.9 and 5.10).
@0xTim Sorry about that, not sure what happened. But all’s good now, I think.
While I am generally loathe to make breaking changes, I’m not sure in this case it’s cut-and-dry. Here are my thoughts.
- When is the next major release?
- The current behavior has bitten at least a few people that I know of.
- 10 ms seems like an impossibly small timeout; where did it originate? The other default timeout is 100 ms, as it meant to be that?
- Even in very high performance systems (where a connection rarely times out), would anyone deliberately rely on such a short timeout? If someone wasn't already working around this by setting a longer timeout, would suddenly having a longer one adversely affect them? I get that we can't say for certain, but what's the likelihood, in a conceivable system, that it would?
At the very least, the documentation and example should make it clear this is an issue.
It's also not a breaking change - it's a change in behaviour for sure but code will still compile which is what SemVer defines as breaking changes. Whether it should be accepted is still a question for the maintainers though