search

valkey-io / valkey-glide

An open source Valkey client library that supports Valkey and Redis open source 6.2, 7.0 and 7.2. Valkey GLIDE is designed for reliability, optimized performance, and high-availability, for Valkey and Redis OSS based applications. GLIDE is a multi language client library, written in Rust with programming language bindings, such as Java and Python
Apache License 2.0
279 stars 58 forks source link

Node: "Invalid endpoints format: Cannot read properties of undefined (reading 'split')" #2624

Closed ikolomi closed 2 weeks ago

ikolomi commented 3 weeks ago

Describe the bug

CI links: https://github.com/valkey-io/valkey-glide/actions/runs/11701901352/job/32588956465

Expected Behavior

test pass

Current Behavior

test fail

Reproduction Steps

manual run or CI

Possible Solution

No response

Additional Information/Context

No response

Client version used

release-1.2

Engine type and version

.

OS

.

Language

TypeScript

Language Version

.

Cluster information

No response

Logs

No response

Other information

No response

Yury-Fridlyand commented 3 weeks ago

Issue still persist https://github.com/valkey-io/valkey-glide/actions/runs/11738351903/job/32700728123?pr=2633

avifenesh commented 3 weeks ago

@Yury-Fridlyand I added --runInBand back, I see that somebody overridden it for some reason. Please add it back. runInBand makes the tests run one by one, removing it cuts the running time by half. However, if tests need to have the CLI args, they can't run on different processes. So, for this case of using a hard-codded cluster passed as a cmd arg, I needed to add it back. Since currently those are a few tests, it's not an issue. If the test set grows, we will have to rethink it. In CI, It's normal to run with runInBand so there's less stress on the machine, in our case it makes a lot of different, and it's not a matter of 2–3 minutes, but from 25 min test run to 12. I profiled behavior to see that we don't have leaks in tests and usage grow, and we don't so we can afford it. Anyhow, in this case it is not possible unless we come up with an idea how to solve it differently.

avifenesh commented 3 weeks ago

Plus — I see the branch is not updated, can you please rebase and run? Maybe nobody added it, its just not updated to latest changes.

Yury-Fridlyand commented 3 weeks ago

will update and observe, but it is a flaky thing. I don't tell that it is not fixed, but to confirm that it is fixed, we need to run CI 100500 times.

avifenesh commented 3 weeks ago

I ran full matrix twice before closing it. But it's not it, I reproduced, understood the issue, which wasn't to understand, tried hard to make it work somehow before re-adding, and after failing hardly I added it back, and stress tested it. I'm working with memdb and all, of course. This one specifically wasn't a flaky, it's me who broke it by removing, and when adding back I fixed.

avifenesh commented 2 weeks ago

@Yury-Fridlyand It didn't reoccur. I'll wait another round and close. Can always reopen later.