You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/10/06 08:21:43 UTC

[GitHub] [camel-quarkus] llowinge opened a new pull request #1877: Fix twitter itest so it initially waits when start polling tweets

llowinge opened a new pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
ppalaga commented on a change in pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#discussion_r500121115



##########
File path: integration-tests/twitter/src/test/java/org/apache/camel/quarkus/component/twitter/CamelTwitterTest.java
##########
@@ -80,8 +80,10 @@ public void e2e() {
         final long sinceId = Long.parseLong(messageId) - 1;
         /* Check that the message is seen in the timeline by the polling consumer */
         {
+            final int initialDelayMs = 60000;
             final int retries = 5;
             final int delayMs = 3000;
+            Thread.sleep(initialDelayMs);

Review comment:
       This will prolong the test execution by 1 min even if the message appears in the timeline much earlier. Wouldn't it be better to increase the number of retries? Or wouldn't some shorter initialDelayMs work too? like 10 seconds?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] ppalaga commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
ppalaga commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704728444


   > Can't we use awaitility to poll the Twitter timeline until it contains the expected message
   
   As far as I understand, the main motivation for this quirks is the request rate limit Twitter imposes in the free tier of their API access. So we need to send as little requests as possible. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] llowinge commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
llowinge commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704874393


   @ppalaga So the outcome of the discussion is we should rewrite it with awaitlity + exposing ENV for selected timeouts ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] ppalaga commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
ppalaga commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704266970


   > I could also make it configurable via ENV
   
   Yeah, that sounds good


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] jamesnetherton merged pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
jamesnetherton merged pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] llowinge commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
llowinge commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704128127


   @jamesnetherton I remember i was trying to achieve it like you suggest in test `direct` but i had some problems that it didn't wait or didin't work at all (don't remember), probably caused by method used `receiveBodyNoWait`, so i had to use (like in https://github.com/apache/camel-quarkus/commit/f251bbf028d48f970b5bf79b867ebdd3cb055402).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] llowinge edited a comment on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
llowinge edited a comment on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704131276


   @jamesnetherton I've tried it and with `initialDelay` set to 60000, it prints 
   ```
   2020-10-06 10:49:37,492 INFO  [org.apa.cam.qua.com.twi.CamelResource] (executor-thread-1) Posted a tweet Hello from camel-quarkus-twitter 8421a742b16c4ad0b6306bd6dcae4c9c
   2020-10-06 10:50:07,541 INFO  [org.apa.cam.qua.com.twi.CamelResource] (executor-thread-1) Received tweets from user's timeline: null
   2020-10-06 10:50:37,842 INFO  [org.apa.cam.qua.com.twi.CamelResource] (executor-thread-1) Received tweets from user's timeline: Tue Oct 06 10:49:37 CEST 2020 (jbossfuseqa) Hello from camel-quarkus-twitter 8421a742b16c4ad0b6306bd6dcae4c9c
   ```
   Which is weird, as i would think it would print every 1min, not 30s.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] jamesnetherton merged pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
jamesnetherton merged pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] llowinge commented on a change in pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
llowinge commented on a change in pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#discussion_r500182344



##########
File path: integration-tests/twitter/src/test/java/org/apache/camel/quarkus/component/twitter/CamelTwitterTest.java
##########
@@ -80,8 +80,10 @@ public void e2e() {
         final long sinceId = Long.parseLong(messageId) - 1;
         /* Check that the message is seen in the timeline by the polling consumer */
         {
+            final int initialDelayMs = 60000;
             final int retries = 5;
             final int delayMs = 3000;
+            Thread.sleep(initialDelayMs);

Review comment:
       @ppalaga Yes, in ideal world, it would be much better, but with such approach it leads to 
   ```
   429:Returned in API v1.1 when a request cannot be served due to the application's rate limit having been exhausted for the resource. See Rate Limiting in API v1.1.(https://dev.twitter.com/docs/rate-limiting/1.1)
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] lburgazzoli commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704737655


   > > Can't we use awaitility to poll the Twitter timeline until it contains the expected message
   > 
   > As far as I understand, the main motivation for this quirks is the request rate limit Twitter imposes in the free tier of their API access. So we need to send as little requests as possible.
   
   AFAIK awaitility has support for [polling](https://github.com/awaitility/awaitility/wiki/Usage#polling) 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] llowinge commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
llowinge commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704190421


   @ppalaga I could also make it configurable via ENV, so i can change in in our test suite to desired amount when needed ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] ppalaga commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
ppalaga commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-705512900


   > @ppalaga So the outcome of the discussion is we should rewrite it with awaitlity + exposing ENV for selected timeouts ?
   
   Yes, awaitlity and ENV with some reasonable defaults would be would be awesome. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] ppalaga commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
ppalaga commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-705512900


   > @ppalaga So the outcome of the discussion is we should rewrite it with awaitlity + exposing ENV for selected timeouts ?
   
   Yes, awaitlity and ENV with some reasonable defaults would be would be awesome. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] ppalaga commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
ppalaga commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704748879


   Oh sorry, I have overseen the stress on awaitility. I thought, it was about simplifying the polling algo by leaving out the initial delay. Of course awaitility would simplify the code. The original impl is from me when I had no idea about awaitility.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] llowinge commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
llowinge commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704131276


   @jamesnetherton I've tried it and with `initialDelay` set to 60000, it prints ```
   2020-10-06 10:49:37,492 INFO  [org.apa.cam.qua.com.twi.CamelResource] (executor-thread-1) Posted a tweet Hello from camel-quarkus-twitter 8421a742b16c4ad0b6306bd6dcae4c9c
   2020-10-06 10:50:07,541 INFO  [org.apa.cam.qua.com.twi.CamelResource] (executor-thread-1) Received tweets from user's timeline: null
   2020-10-06 10:50:37,842 INFO  [org.apa.cam.qua.com.twi.CamelResource] (executor-thread-1) Received tweets from user's timeline: Tue Oct 06 10:49:37 CEST 2020 (jbossfuseqa) Hello from camel-quarkus-twitter 8421a742b16c4ad0b6306bd6dcae4c9c
   ```
   Which is weird, as i would think it would print every 1min, not 30s.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-quarkus] jamesnetherton commented on pull request #1877: Fix twitter itest so it initially waits when start polling tweets

Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on pull request #1877:
URL: https://github.com/apache/camel-quarkus/pull/1877#issuecomment-704724968


   > > I could also make it configurable via ENV
   > 
   > Yeah, that sounds good
   
   Can't we use awaitility to poll the Twitter timeline until it contains the expected message? It'd be better than an arbitrary sleep value & env var.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org