[jira] [Commented] (IGNITE-4226) Redis SET command should handle expirations
[ https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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
[ https://issues.apache.org/jira/browse/IGNITE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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)