Added missing idle args in XPendingExtArgs

[parvez0]

Made changes to XPendingExt method for allowing the usage of idle argument if defined in XPendingExtArgs

[monkey92t]

@parvez0

Hi~ this modification is wrong, you need to check the order of the parameters, redis-server does not handle it rigorously.
https://github.com/redis/redis/blob/3d56be3788a68d81ddb259197b0b2f4149bd94cb/src/t_stream.c#L2573-L2630

In addition, we need to complete it in the unit test

[monkey92t]

For the time type, time.Duration is generally used in golang to handle it.

[parvez0]

What is the command for running test and also which platform is it supported by make test. If you can point me to a developer guide that will be great. As for those changes I'll do it thanks for the clarification

[monkey92t]

You can follow github workflow, link: https://docs.github.com/en/actions/learn-github-actions/finding-and-customizing-actions

[parvez0]

I have updated the pull request can you verify it also I think for the first time I'll need the approval to run workflows

[monkey92t]

According to the redis documentation, the unit of idle is milliseconds, not nanoseconds

[parvez0]

I have looked at the MinRetryBackoff and other fields there is no conversion which is happening for time. Duration if specified I thought the developer has to take care of that setting so should I change it so that at the time of adding it should convert it to millisec

[parvez0]

@monkey92t can you verify the changes now

[monkey92t]

should be: args = append(args, "idle", formatMs(ctx, a.Idle))

[parvez0]

formatMs should be a new function right where should it go I mean in which file

[monkey92t]

https://github.com/go-redis/redis/blob/7d319aeb5735d0f2fef4e513bb65c7f1ce9e38a8/commands.go#L22-L32

[monkey92t]

If we can add an effective idle test, it will be good

[parvez0]

Sure I'll write a separate test case for Idle time

[parvez0]

@monkey92t sorry for bugging you, I have written the test case but in order to verify it I have to add a simple time.Sleep in between of the test cases and I am not sure if that is the right way to do it. I just wanted to confirm once

[parvez0]

			// sleeping for 2 millisecond to test for idle time
			time.Sleep(5 * time.Millisecond)
			infoExt, err = client.XPendingExt(ctx, &redis.XPendingExtArgs{
                                     // other fields
				Idle:     2 * time.Millisecond,
			}).Result()

[parvez0]

and also there are some linting errors from my previous commit which is breaking the checks on mine can you tell what should I do about that

[monkey92t]

We only need to set a large enough time to prove the normal execution of the idle parameter.
500 milliseconds is too short, it is easy to make mistakes

args.Idle = 72 * time.Hour
infoExt, err = client.XPendingExt(ctx, args).Result()
Expect(err).NotTo(HaveOccurred())
Expect(infoExt).To(HaveLen(0))

[monkey92t]

Fixed #1749

[jalmeter]

@monkey92t - Do you have a guess for when this will be released?

[vmihailenco]

I've pushed v8.8.3