You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@bahir.apache.org by ariskk <gi...@git.apache.org> on 2017/03/14 10:17:27 UTC

[GitHub] bahir-flink pull request #13: [BAHIR-95] Add ZREM to Redis commands

GitHub user ariskk opened a pull request:

    https://github.com/apache/bahir-flink/pull/13

    [BAHIR-95] Add ZREM to Redis commands

    Adds ZREM.
    Unless there is a really good reason not to, i intend to add a few others as well,
    


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

    $ git pull https://github.com/ariskk/bahir-flink bahir-95

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

    https://github.com/apache/bahir-flink/pull/13.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 #13
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bahir-flink pull request #13: [BAHIR-95] Add ZREM to Redis commands

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/bahir-flink/pull/13


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bahir-flink issue #13: [BAHIR-95] Add ZREM to Redis commands

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the issue:

    https://github.com/apache/bahir-flink/pull/13
  
    I think the reason why `ZREM` was not added is because this is the `RedisSink`, which implies that you are writing data to redis, not removing it.
    I'm not sure if it makes sense semantically. Do you have a use case for removing data in the sink?
    
    The change looks good overall.
    Maybe you could also update the `README.md` file with the new command?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bahir-flink issue #13: [BAHIR-95] Add ZREM to Redis commands

Posted by ariskk <gi...@git.apache.org>.
Github user ariskk commented on the issue:

    https://github.com/apache/bahir-flink/pull/13
  
    In our use case, both additions and deletions are processed via flink.
    Think of a graph for example, that you store nodes as keys and their edges as sorted sets (eg sorted on date created). Assuming this is dynamic, edges may be added, but may also be removed.
    I agree it wouldn't make much sense to delete keys, but the ability to add and remove elements from sets is important.
    We are using our own Redis Sink up to now, which supports this, but we are thinking to migrate to the official one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] bahir-flink issue #13: [BAHIR-95] Add ZREM to Redis commands

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the issue:

    https://github.com/apache/bahir-flink/pull/13
  
    Thank you for updating the PR.
    
    I'll merge it.
    
    I'm planning to create the first bahir-flink release soon. Would you like to first add more commands to the connector, or is it good to go for a first release?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---