[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15877424#comment-15877424
 ] 

ASF GitHub Bot commented on IGNITE-4226:


Github user asfgit closed the pull request at:

https://github.com/apache/ignite/pull/1510


> Redis SET command should handle expirations
> ---
>
> Key: IGNITE-4226
> URL: https://issues.apache.org/jira/browse/IGNITE-4226
> Project: Ignite
>  Issue Type: Sub-task
>Affects Versions: 1.8
>Reporter: Roman Shtykh
>Assignee: Roman Shtykh
>  Labels: redis
> Fix For: 2.0
>
>
> Handling EX and PX parameters of SET command.
> https://redis.io/commands/set



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations

[Roman Shtykh (JIRA)]

[ 
https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15877373#comment-15877373
 ] 

Roman Shtykh commented on IGNITE-4226:
--

[~anovikov] You are right. Please see the changes.


> Redis SET command should handle expirations
> ---
>
> Key: IGNITE-4226
> URL: https://issues.apache.org/jira/browse/IGNITE-4226
> Project: Ignite
>  Issue Type: Sub-task
>Affects Versions: 1.8
>Reporter: Roman Shtykh
>Assignee: Roman Shtykh
>  Labels: redis
> Fix For: 2.0
>
>
> Handling EX and PX parameters of SET command.
> https://redis.io/commands/set



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations

[Andrey Novikov (JIRA)]

[ 
https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15875519#comment-15875519
 ] 

Andrey Novikov commented on IGNITE-4226:


Hi [~roman_s],

{{setExpire}} look dirty and contains many duplicated code, non-informative 
error messages. I don't think that we receive performance drop if we iterate 
over list of 5-10 items twice.

> Redis SET command should handle expirations
> ---
>
> Key: IGNITE-4226
> URL: https://issues.apache.org/jira/browse/IGNITE-4226
> Project: Ignite
>  Issue Type: Sub-task
>Affects Versions: 1.8
>Reporter: Roman Shtykh
>Assignee: Roman Shtykh
>  Labels: redis
> Fix For: 2.0
>
>
> Handling EX and PX parameters of SET command.
> https://redis.io/commands/set



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations

[Roman Shtykh (JIRA)]

[ 
https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15871325#comment-15871325
 ] 

Roman Shtykh commented on IGNITE-4226:
--

Hi [~anovikov],

Thank you for pointing to {{IgniteUtils#containsStringCollection}}! Modified.

As for creating {{GridRedisRestCommandHandler#longValue}}, it looks cleaner, 
but then you would have to iterate the same parameter list twice -- for _px_ 
and the second time for _ex_, like 
{code}
Long px = longValue("px", params);
Long ex = longValue("ex", params);

if (px != null)
  restReq.ttl(px);
else if (ex != null)
  restReq.ttl(ex * 1000L);
{code}
And the default value is not really needed here.

I propose to leave {{setExpire}} as is, if there are no concerns, and 
generalize long value retrieval later when we have more code handling similar 
things.

> Redis SET command should handle expirations
> ---
>
> Key: IGNITE-4226
> URL: https://issues.apache.org/jira/browse/IGNITE-4226
> Project: Ignite
>  Issue Type: Sub-task
>Affects Versions: 1.8
>Reporter: Roman Shtykh
>Assignee: Roman Shtykh
>  Labels: redis
> Fix For: 2.0
>
>
> Handling EX and PX parameters of SET command.
> https://redis.io/commands/set



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations

[Andrey Novikov (JIRA)]

[ 
https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15869766#comment-15869766
 ] 

Andrey Novikov commented on IGNITE-4226:


Hi [~roman_s],

Sorry, it was my mistake: GridRedisSetCommandHandler#isNx and 
GridRedisSetCommandHandler#isXx can be replaced by 
IgniteUtils#containsStringCollection

You may define method in GridRedisRestCommandHandler like this:
{code}
/**
 * Retrieves long value from parameters list.
 *
 * @param key Key.
 * @param params Parameters list.
 * @param dfltVal Default value.
 * @return Long value from parameters list or {@code dfltVal} if null or not 
exists.
 * @throws GridRedisGenericException If parsing failed.
 */
@Nullable protected Long longValue(String key, List params,
Long dfltVal) throws IgniteCheckedException {
assert key != null;

Iterator it = params.iterator();

while (it.hasNext()) {
String opt = it.next();

if (key.equalsIgnoreCase(opt)) {
if (it.hasNext()) {
String val = it.next();

try {
return Long.valueOf(val);
}
catch (NumberFormatException ignore) {
throw new GridRedisGenericException("Failed to parse 
parameter of Long type [" + key + "=" + val + "]");
}
}
else
throw new GridRedisGenericException("Missed value for 
parameter: " + key);
}
}
}
{code}
And remove parameters parsing from GridRedisSetCommandHandler#setExpire.

> Redis SET command should handle expirations
> ---
>
> Key: IGNITE-4226
> URL: https://issues.apache.org/jira/browse/IGNITE-4226
> Project: Ignite
>  Issue Type: Sub-task
>Affects Versions: 1.8
>Reporter: Roman Shtykh
>Assignee: Roman Shtykh
>  Labels: redis
> Fix For: 2.0
>
>
> Handling EX and PX parameters of SET command.
> https://redis.io/commands/set



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations

[Roman Shtykh (JIRA)]

[ 
https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15869662#comment-15869662
 ] 

Roman Shtykh commented on IGNITE-4226:
--

Hi [~anovikov],

Thank you for checking!
But {{GridJettyRestHandler#longValue}} is not a core module. Why would we 
reference it from core?

Also, {{GridRedisSetCommandHandler#isNx}} and 
{{GridRedisSetCommandHandler#isXx}} cannot be replaced. These are the methods 
that has nothing to do with expiration, I just modified them to work with _EX_ 
and _PX_ options.

> Redis SET command should handle expirations
> ---
>
> Key: IGNITE-4226
> URL: https://issues.apache.org/jira/browse/IGNITE-4226
> Project: Ignite
>  Issue Type: Sub-task
>Affects Versions: 1.8
>Reporter: Roman Shtykh
>Assignee: Roman Shtykh
>  Labels: redis
> Fix For: 2.0
>
>
> Handling EX and PX parameters of SET command.
> https://redis.io/commands/set



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations

[Andrey Novikov (JIRA)]

[ 
https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15869576#comment-15869576
 ] 

Andrey Novikov commented on IGNITE-4226:


Hi [~roman_s],

I have comment on the fix:
GridRedisSetCommandHandler#isNx, GridRedisSetCommandHandler#isXx, 
GridRedisSetCommandHandler#setExpire may be replaced by 
GridJettyRestHandler#longValue

The rest looks good to me.

> Redis SET command should handle expirations
> ---
>
> Key: IGNITE-4226
> URL: https://issues.apache.org/jira/browse/IGNITE-4226
> Project: Ignite
>  Issue Type: Sub-task
>Affects Versions: 1.8
>Reporter: Roman Shtykh
>Assignee: Roman Shtykh
>  Labels: redis
> Fix For: 2.0
>
>
> Handling EX and PX parameters of SET command.
> https://redis.io/commands/set



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15858895#comment-15858895
 ] 

ASF GitHub Bot commented on IGNITE-4226:


GitHub user shroman opened a pull request:

https://github.com/apache/ignite/pull/1510

IGNITE-4226: Redis SET command to handle expirations.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/shroman/ignite IGNITE-4226

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/ignite/pull/1510.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1510


commit 4642dbf8b47205cb15901f4a9deb6bc984959261
Author: shtykh_roman 
Date:   2017-02-09T02:27:31Z

IGNITE-4226: Redis SET command to handle expirations.




> Redis SET command should handle expirations
> ---
>
> Key: IGNITE-4226
> URL: https://issues.apache.org/jira/browse/IGNITE-4226
> Project: Ignite
>  Issue Type: Sub-task
>Affects Versions: 1.8
>Reporter: Roman Shtykh
>Assignee: Roman Shtykh
>  Labels: redis
> Fix For: 1.9
>
>




--
This message was sent by Atlassian JIRA
(v6.3.15#6346)