You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@streams.apache.org by rbnks <gi...@git.apache.org> on 2014/07/11 23:33:05 UTC
[GitHub] incubator-streams pull request: Streams 121
GitHub user rbnks opened a pull request:
https://github.com/apache/incubator-streams/pull/53
Streams 121
Instagram Recent Media provider and tests. A serialization test was deleted out due to testing errors and it was not fully implemented.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rbnks/incubator-streams STREAMS-121
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-streams/pull/53.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 #53
----
commit 77603934e41ac78ba68f73b3f80ea6852f0acb46
Author: rebanks <re...@w2odigital.com>
Date: 2014-07-11T21:09:59Z
STREAMS 121 | Added instagram provider and tests
commit 95f6d5d9bc9069258717a9880e491ce55ae343e2
Author: rebanks <re...@w2odigital.com>
Date: 2014-07-11T21:29:55Z
STREAMS 121 | Added javadoc comments
----
---
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: Streams 121
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on a diff in the pull request:
https://github.com/apache/incubator-streams/pull/53#discussion_r14878711
--- Diff: streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/provider/InstagramRecentMediaProvider.java ---
@@ -0,0 +1,101 @@
+package org.apache.streams.instagram.provider;
+
+import com.google.common.collect.Queues;
+import org.apache.streams.config.StreamsConfigurator;
+import org.apache.streams.core.StreamsDatum;
+import org.apache.streams.core.StreamsProvider;
+import org.apache.streams.core.StreamsResultSet;
+import org.apache.streams.instagram.InstagramConfigurator;
+import org.apache.streams.instagram.InstagramUserInformationConfiguration;
+import org.jinstagram.entity.users.feed.MediaFeedData;
+import org.joda.time.DateTime;
+
+import java.math.BigInteger;
+import java.util.Queue;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Instagram {@link org.apache.streams.core.StreamsProvider} that provides the recent media data for a group of users
+ */
+public class InstagramRecentMediaProvider implements StreamsProvider {
+
+ private InstagramUserInformationConfiguration config;
+ private InstagramRecentMediaCollector dataCollector;
+ protected Queue<MediaFeedData> mediaFeedQueue; //exposed for testing
+ private ExecutorService executorService;
+ private volatile boolean isCompleted;
--- End diff --
use an AtomicBoolean 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: Streams 121
Posted by rbnks <gi...@git.apache.org>.
Github user rbnks commented on the pull request:
https://github.com/apache/incubator-streams/pull/53#issuecomment-48785614
Fixing merge conflicts.....
---
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: Streams 121
Posted by rbnks <gi...@git.apache.org>.
Github user rbnks commented on the pull request:
https://github.com/apache/incubator-streams/pull/53#issuecomment-48913822
Added license headers. The original pull request comments still show up though. but they are there.
---
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: Streams 121
Posted by robdouglas <gi...@git.apache.org>.
Github user robdouglas commented on the pull request:
https://github.com/apache/incubator-streams/pull/53#issuecomment-48956827
:+1:
---
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: Streams 121
Posted by rbnks <gi...@git.apache.org>.
Github user rbnks commented on the pull request:
https://github.com/apache/incubator-streams/pull/53#issuecomment-48945236
Pull request updated with change from volatile to atomic.
---
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: Streams 121
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on a diff in the pull request:
https://github.com/apache/incubator-streams/pull/53#discussion_r14884175
--- Diff: streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/provider/InstagramRecentMediaProvider.java ---
@@ -0,0 +1,101 @@
+package org.apache.streams.instagram.provider;
+
+import com.google.common.collect.Queues;
+import org.apache.streams.config.StreamsConfigurator;
+import org.apache.streams.core.StreamsDatum;
+import org.apache.streams.core.StreamsProvider;
+import org.apache.streams.core.StreamsResultSet;
+import org.apache.streams.instagram.InstagramConfigurator;
+import org.apache.streams.instagram.InstagramUserInformationConfiguration;
+import org.jinstagram.entity.users.feed.MediaFeedData;
+import org.joda.time.DateTime;
+
+import java.math.BigInteger;
+import java.util.Queue;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Instagram {@link org.apache.streams.core.StreamsProvider} that provides the recent media data for a group of users
+ */
+public class InstagramRecentMediaProvider implements StreamsProvider {
+
+ private InstagramUserInformationConfiguration config;
+ private InstagramRecentMediaCollector dataCollector;
+ protected Queue<MediaFeedData> mediaFeedQueue; //exposed for testing
+ private ExecutorService executorService;
+ private volatile boolean isCompleted;
--- End diff --
True that it isn't required in this context, and that a non-thread-local cached value will be fine, but the Atomic pattern is widely used throughout the codebase and the minor locking in Atomic will have no performance implication
---
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: Streams 121
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-streams/pull/53
---
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: Streams 121
Posted by rbnks <gi...@git.apache.org>.
Github user rbnks commented on the pull request:
https://github.com/apache/incubator-streams/pull/53#issuecomment-48786488
Auto merge ok. Added back in the test, but it is passing now.
---
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: Streams 121
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on a diff in the pull request:
https://github.com/apache/incubator-streams/pull/53#discussion_r14878761
--- Diff: streams-contrib/streams-provider-instagram/src/test/java/org/apache/streams/instagram/provider/InstagramRecentMediaProviderTest.java ---
@@ -0,0 +1,145 @@
+package org.apache.streams.instagram.provider;
--- End diff --
license header
---
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: Streams 121
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on a diff in the pull request:
https://github.com/apache/incubator-streams/pull/53#discussion_r14878395
--- Diff: streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/provider/InstagramRecentMediaCollector.java ---
@@ -0,0 +1,150 @@
+package org.apache.streams.instagram.provider;
--- End diff --
This needs an Apache license header
---
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: Streams 121
Posted by rbnks <gi...@git.apache.org>.
Github user rbnks commented on a diff in the pull request:
https://github.com/apache/incubator-streams/pull/53#discussion_r14882859
--- Diff: streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/provider/InstagramRecentMediaProvider.java ---
@@ -0,0 +1,101 @@
+package org.apache.streams.instagram.provider;
+
+import com.google.common.collect.Queues;
+import org.apache.streams.config.StreamsConfigurator;
+import org.apache.streams.core.StreamsDatum;
+import org.apache.streams.core.StreamsProvider;
+import org.apache.streams.core.StreamsResultSet;
+import org.apache.streams.instagram.InstagramConfigurator;
+import org.apache.streams.instagram.InstagramUserInformationConfiguration;
+import org.jinstagram.entity.users.feed.MediaFeedData;
+import org.joda.time.DateTime;
+
+import java.math.BigInteger;
+import java.util.Queue;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Instagram {@link org.apache.streams.core.StreamsProvider} that provides the recent media data for a group of users
+ */
+public class InstagramRecentMediaProvider implements StreamsProvider {
+
+ private InstagramUserInformationConfiguration config;
+ private InstagramRecentMediaCollector dataCollector;
+ protected Queue<MediaFeedData> mediaFeedQueue; //exposed for testing
+ private ExecutorService executorService;
+ private volatile boolean isCompleted;
--- End diff --
Are you referring to line 28? A volatile variable is all that is need here because it is only read and written too. There are no read then updates. http://www.javamex.com/tutorials/synchronization_volatile.shtml
---
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: Streams 121
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on the pull request:
https://github.com/apache/incubator-streams/pull/53#issuecomment-48949236
:+1:
---
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: Streams 121
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on a diff in the pull request:
https://github.com/apache/incubator-streams/pull/53#discussion_r14878373
--- Diff: streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/InstagramConfigurator.java ---
@@ -41,8 +38,8 @@
public static InstagramConfiguration detectInstagramConfiguration(Config config) {
- ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
- Validator validator = factory.getValidator();
+// ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
--- End diff --
Remove or fix rather than comment out
---
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: Streams 121
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on a diff in the pull request:
https://github.com/apache/incubator-streams/pull/53#discussion_r14878752
--- Diff: streams-contrib/streams-provider-instagram/src/test/java/org/apache/streams/instagram/provider/InstagramRecentMediaCollectorTest.java ---
@@ -0,0 +1,161 @@
+package org.apache.streams.instagram.provider;
--- End diff --
license header
---
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.
---