You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@streams.apache.org by steveblackmon <gi...@git.apache.org> on 2015/10/14 23:10:35 UTC

[GitHub] incubator-streams pull request: resolves STREAMS-371

GitHub user steveblackmon opened a pull request:

    https://github.com/apache/incubator-streams/pull/263

    resolves STREAMS-371

    

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

    $ git pull https://github.com/steveblackmon/incubator-streams STREAMS-371

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

    https://github.com/apache/incubator-streams/pull/263.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 #263
    
----
commit 20e7b66171634b948daaa6d9cc119990fbeec50f
Author: Steve Blackmon <sb...@apache.org>
Date:   2015-10-14T20:36:44Z

    resolves STREAMS-371

----


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42414929
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -144,6 +165,57 @@ else if( endpoint.equals("friends") )
             } while (curser != 0 && keepTrying < 10);
         }
     
    +    private void collectIds(Long id) {
    +        int keepTrying = 0;
    +
    +        long curser = -1;
    +
    +        do
    +        {
    +            try
    +            {
    +                twitter4j.IDs ids = null;
    +                if( endpoint.equals("followers") )
    +                    ids = client.friendsFollowers().getFollowersIDs(id.longValue(), curser, max_per_page);
    +                else if( endpoint.equals("friends") )
    +                    ids = client.friendsFollowers().getFriendsIDs(id.longValue(), curser, max_per_page);
    +
    +                Preconditions.checkNotNull(ids);
    +                Preconditions.checkArgument(ids.getIDs().length > 0);
    +
    +                for (long otherId : ids.getIDs()) {
    +
    +                    try {
    +                        Follow follow;
    +                        if( endpoint.equals("followers") ) {
    +                            follow = new Follow()
    +                                    .withFollowee(new User().withId(id))
    +                                    .withFollower(new User().withId(otherId));
    +                        } else if( endpoint.equals("friends") ) {
    +                            follow = new Follow()
    +                                    .withFollowee(new User().withId(otherId))
    +                                    .withFollower(new User().withId(id));
    +                        } else {
    +                            throw new Exception("endpoint must be set to 'friends' or 'followers'");
    --- End diff --
    
    the precondition was already being checked, just outside the method where the compiler couldn't trace.  changed to a not null precondition here.


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by eponvert <gi...@git.apache.org>.
Github user eponvert commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42375094
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProvider.java ---
    @@ -129,7 +134,7 @@ public StreamsResultSet readCurrent() {
     
         @Override
         public void prepare(Object o) {
    --- End diff --
    
    Not really clear what this method does, but it doesn't seem to do anything with its Object o parameter any more. Should this be removed?


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42414637
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -86,6 +89,13 @@ protected void getFollowing(Long id) {
     
             Preconditions.checkArgument(endpoint.equals("friends") || endpoint.equals("followers"));
     
    +        if( ids_only == true )
    --- End diff --
    
    changed


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by eponvert <gi...@git.apache.org>.
Github user eponvert commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42375565
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -86,6 +89,13 @@ protected void getFollowing(Long id) {
     
             Preconditions.checkArgument(endpoint.equals("friends") || endpoint.equals("followers"));
     
    +        if( ids_only == true )
    --- End diff --
    
    why not just say
    ```java
    if (ids_only)
    ```



---
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] incubator-streams pull request: resolves STREAMS-371

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42413877
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProvider.java ---
    @@ -129,7 +134,7 @@ public StreamsResultSet readCurrent() {
     
         @Override
         public void prepare(Object o) {
    --- End diff --
    
    It's delegating prepare() to it's parent class and opting to use the configuration provided by the constructor rather than by the streams runtime.  prepare(Object) is defined by the interface and thus has to be implemented, but exact implementation details vary.


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by eponvert <gi...@git.apache.org>.
Github user eponvert commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42376204
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -144,6 +165,57 @@ else if( endpoint.equals("friends") )
             } while (curser != 0 && keepTrying < 10);
         }
     
    +    private void collectIds(Long id) {
    +        int keepTrying = 0;
    +
    +        long curser = -1;
    +
    +        do
    +        {
    +            try
    +            {
    +                twitter4j.IDs ids = null;
    +                if( endpoint.equals("followers") )
    +                    ids = client.friendsFollowers().getFollowersIDs(id.longValue(), curser, max_per_page);
    +                else if( endpoint.equals("friends") )
    +                    ids = client.friendsFollowers().getFriendsIDs(id.longValue(), curser, max_per_page);
    +
    +                Preconditions.checkNotNull(ids);
    +                Preconditions.checkArgument(ids.getIDs().length > 0);
    --- End diff --
    
    Similarly, don't you probably want to know whether it was `client.friendsFollowers()` or `client.friendsFollowers()` that produced empty results, rather than checking this variable here?


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42414302
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -48,23 +48,26 @@
         protected TwitterFollowingProvider provider;
         protected Twitter client;
         protected Long id;
    +    protected Boolean ids_only;
    --- End diff --
    
    chose ids_only as a name to match the corresponding json parameter.  but idsOnly is actually a closer match the configuration bean, so i'll change to that.


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by eponvert <gi...@git.apache.org>.
Github user eponvert commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42376075
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -144,6 +165,57 @@ else if( endpoint.equals("friends") )
             } while (curser != 0 && keepTrying < 10);
         }
     
    +    private void collectIds(Long id) {
    +        int keepTrying = 0;
    +
    +        long curser = -1;
    +
    +        do
    +        {
    +            try
    +            {
    +                twitter4j.IDs ids = null;
    +                if( endpoint.equals("followers") )
    +                    ids = client.friendsFollowers().getFollowersIDs(id.longValue(), curser, max_per_page);
    +                else if( endpoint.equals("friends") )
    +                    ids = client.friendsFollowers().getFriendsIDs(id.longValue(), curser, max_per_page);
    +
    +                Preconditions.checkNotNull(ids);
    --- End diff --
    
    This is a little roundabout, right? The conditions where `ids == null` is where either `!endpoint.equals("followers")` or `!endpoint.equals("friends")`. Really the Preconditions should be checking *that*, and throwing a more verbose exception in the case it fails.


---
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] incubator-streams pull request: resolves STREAMS-371

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

    https://github.com/apache/incubator-streams/pull/263


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by eponvert <gi...@git.apache.org>.
Github user eponvert commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42375321
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -48,23 +48,26 @@
         protected TwitterFollowingProvider provider;
         protected Twitter client;
         protected Long id;
    +    protected Boolean ids_only;
    --- End diff --
    
    Why the non-standard Java variable format?


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42415135
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -144,6 +165,57 @@ else if( endpoint.equals("friends") )
             } while (curser != 0 && keepTrying < 10);
         }
     
    +    private void collectIds(Long id) {
    +        int keepTrying = 0;
    +
    +        long curser = -1;
    +
    +        do
    +        {
    +            try
    +            {
    +                twitter4j.IDs ids = null;
    +                if( endpoint.equals("followers") )
    +                    ids = client.friendsFollowers().getFollowersIDs(id.longValue(), curser, max_per_page);
    +                else if( endpoint.equals("friends") )
    +                    ids = client.friendsFollowers().getFriendsIDs(id.longValue(), curser, max_per_page);
    +
    +                Preconditions.checkNotNull(ids);
    --- End diff --
    
    There can be zero ids in several cases, for example when the profile is private or has zero friends/followers.  so checking the size is the appropriate filter before proceeding


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by eponvert <gi...@git.apache.org>.
Github user eponvert commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42376324
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -144,6 +165,57 @@ else if( endpoint.equals("friends") )
             } while (curser != 0 && keepTrying < 10);
         }
     
    +    private void collectIds(Long id) {
    +        int keepTrying = 0;
    +
    +        long curser = -1;
    +
    +        do
    +        {
    +            try
    +            {
    +                twitter4j.IDs ids = null;
    +                if( endpoint.equals("followers") )
    +                    ids = client.friendsFollowers().getFollowersIDs(id.longValue(), curser, max_per_page);
    +                else if( endpoint.equals("friends") )
    +                    ids = client.friendsFollowers().getFriendsIDs(id.longValue(), curser, max_per_page);
    +
    +                Preconditions.checkNotNull(ids);
    +                Preconditions.checkArgument(ids.getIDs().length > 0);
    +
    +                for (long otherId : ids.getIDs()) {
    +
    +                    try {
    +                        Follow follow;
    +                        if( endpoint.equals("followers") ) {
    +                            follow = new Follow()
    +                                    .withFollowee(new User().withId(id))
    +                                    .withFollower(new User().withId(otherId));
    +                        } else if( endpoint.equals("friends") ) {
    +                            follow = new Follow()
    +                                    .withFollowee(new User().withId(otherId))
    +                                    .withFollower(new User().withId(id));
    +                        } else {
    +                            throw new Exception("endpoint must be set to 'friends' or 'followers'");
    --- End diff --
    
    You should do this check higher up, then not have to worry about throwing an exception here.


---
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] incubator-streams pull request: resolves STREAMS-371

Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on a diff in the pull request:

    https://github.com/apache/incubator-streams/pull/263#discussion_r42415297
  
    --- Diff: streams-contrib/streams-provider-twitter/src/main/java/org/apache/streams/twitter/provider/TwitterFollowingProviderTask.java ---
    @@ -144,6 +165,57 @@ else if( endpoint.equals("friends") )
             } while (curser != 0 && keepTrying < 10);
         }
     
    +    private void collectIds(Long id) {
    +        int keepTrying = 0;
    +
    +        long curser = -1;
    +
    +        do
    +        {
    +            try
    +            {
    +                twitter4j.IDs ids = null;
    +                if( endpoint.equals("followers") )
    +                    ids = client.friendsFollowers().getFollowersIDs(id.longValue(), curser, max_per_page);
    +                else if( endpoint.equals("friends") )
    +                    ids = client.friendsFollowers().getFriendsIDs(id.longValue(), curser, max_per_page);
    +
    +                Preconditions.checkNotNull(ids);
    +                Preconditions.checkArgument(ids.getIDs().length > 0);
    --- End diff --
    
    the results are the same checking the result object - and avoids making the call twice.


---
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.
---