[GitHub] [camel] jmerljak opened a new pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] jmerljak opened a new pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox

jmerljak opened a new pull request #4876:
URL: https://github.com/apache/camel/pull/4876


   Implementation of basic Google Calendar event [synchronization](https://developers.google.com/calendar/v3/sync) for google-calendar-stream component.
   
   See [CAMEL-15961](https://issues.apache.org/jira/browse/CAMEL-15961)


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] oscerd commented on a change in pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox

oscerd commented on a change in pull request #4876:
URL: https://github.com/apache/camel/pull/4876#discussion_r556400607



##########
File path: components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/stream/GoogleCalendarStreamConsumer.java
##########
@@ -59,19 +70,21 @@ public GoogleCalendarStreamEndpoint getEndpoint() {
 
     @Override
     protected int poll() throws Exception {
-        com.google.api.services.calendar.Calendar.Events.List request
-                = getClient().events().list(getConfiguration().getCalendarId()).setOrderBy("updated");
+        Calendar.Events.List request = getClient().events().list(getConfiguration().getCalendarId());
         if (ObjectHelper.isNotEmpty(getConfiguration().getQuery())) {

Review comment:
       Please don't use this class, we have all the tools to check this stuff the camel way

##########
File path: components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/stream/GoogleCalendarStreamConsumer.java
##########
@@ -59,19 +70,21 @@ public GoogleCalendarStreamEndpoint getEndpoint() {
 
     @Override
     protected int poll() throws Exception {
-        com.google.api.services.calendar.Calendar.Events.List request
-                = getClient().events().list(getConfiguration().getCalendarId()).setOrderBy("updated");
+        Calendar.Events.List request = getClient().events().list(getConfiguration().getCalendarId());
         if (ObjectHelper.isNotEmpty(getConfiguration().getQuery())) {
+            Preconditions.checkArgument(!getConfiguration().isSyncFlow(), "query is incompatible with sync flow.");
             request.setQ(getConfiguration().getQuery());
         }
         if (ObjectHelper.isNotEmpty(getConfiguration().getMaxResults())) {
             request.setMaxResults(getConfiguration().getMaxResults());
         }
-        if (getConfiguration().isConsumeFromNow()) {
+        // in synchronization flow only set timeMin on first request
+        if (getConfiguration().isConsumeFromNow() && syncToken == null) {
             Date date = new Date();
             request.setTimeMin(new DateTime(date));
         }
         if (getConfiguration().isConsiderLastUpdate()) {
+            Preconditions.checkArgument(!getConfiguration().isSyncFlow(), "considerLastUpdate is incompatible with sync flow.");

Review comment:
       Same




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] jmerljak commented on a change in pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox
In reply to this post by GitBox

jmerljak commented on a change in pull request #4876:
URL: https://github.com/apache/camel/pull/4876#discussion_r556461158



##########
File path: components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/stream/GoogleCalendarStreamConsumer.java
##########
@@ -59,19 +70,21 @@ public GoogleCalendarStreamEndpoint getEndpoint() {
 
     @Override
     protected int poll() throws Exception {
-        com.google.api.services.calendar.Calendar.Events.List request
-                = getClient().events().list(getConfiguration().getCalendarId()).setOrderBy("updated");
+        Calendar.Events.List request = getClient().events().list(getConfiguration().getCalendarId());
         if (ObjectHelper.isNotEmpty(getConfiguration().getQuery())) {

Review comment:
       I guess this refers to `Preconditions`? What is the Camel way? Could you point me in the right direction?




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] oscerd commented on a change in pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox
In reply to this post by GitBox

oscerd commented on a change in pull request #4876:
URL: https://github.com/apache/camel/pull/4876#discussion_r556467126



##########
File path: components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/stream/GoogleCalendarStreamConsumer.java
##########
@@ -59,19 +70,21 @@ public GoogleCalendarStreamEndpoint getEndpoint() {
 
     @Override
     protected int poll() throws Exception {
-        com.google.api.services.calendar.Calendar.Events.List request
-                = getClient().events().list(getConfiguration().getCalendarId()).setOrderBy("updated");
+        Calendar.Events.List request = getClient().events().list(getConfiguration().getCalendarId());
         if (ObjectHelper.isNotEmpty(getConfiguration().getQuery())) {

Review comment:
       Use ObjectHelper class, check if the syncFlow is not empty and log a warning, I don't think we have to throw an exception.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] jmerljak commented on a change in pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox
In reply to this post by GitBox

jmerljak commented on a change in pull request #4876:
URL: https://github.com/apache/camel/pull/4876#discussion_r556504440



##########
File path: components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/stream/GoogleCalendarStreamConsumer.java
##########
@@ -59,19 +70,21 @@ public GoogleCalendarStreamEndpoint getEndpoint() {
 
     @Override
     protected int poll() throws Exception {
-        com.google.api.services.calendar.Calendar.Events.List request
-                = getClient().events().list(getConfiguration().getCalendarId()).setOrderBy("updated");
+        Calendar.Events.List request = getClient().events().list(getConfiguration().getCalendarId());
         if (ObjectHelper.isNotEmpty(getConfiguration().getQuery())) {

Review comment:
       I was referring to https://github.com/apache/camel/blob/master/components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/GoogleCalendarEndpoint.java#L66 where an exception is thrown when incompatible option is set. Invalid configuration results in request failures, so it's better to fail fast.
   
   Moreover, `syncFlow` may not be empty, it just shouldn't be set to `true`.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] jmerljak commented on a change in pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox
In reply to this post by GitBox

jmerljak commented on a change in pull request #4876:
URL: https://github.com/apache/camel/pull/4876#discussion_r556504440



##########
File path: components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/stream/GoogleCalendarStreamConsumer.java
##########
@@ -59,19 +70,21 @@ public GoogleCalendarStreamEndpoint getEndpoint() {
 
     @Override
     protected int poll() throws Exception {
-        com.google.api.services.calendar.Calendar.Events.List request
-                = getClient().events().list(getConfiguration().getCalendarId()).setOrderBy("updated");
+        Calendar.Events.List request = getClient().events().list(getConfiguration().getCalendarId());
         if (ObjectHelper.isNotEmpty(getConfiguration().getQuery())) {

Review comment:
       I was referring to https://github.com/apache/camel/blob/master/components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/GoogleCalendarEndpoint.java#L66 where an exception is thrown when incompatible option is set. Invalid configuration results in request failures, so it's better to fail fast.
   
   Moreover, `syncFlow` may be set, it just shouldn't be set to `true`.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] oscerd commented on a change in pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox
In reply to this post by GitBox

oscerd commented on a change in pull request #4876:
URL: https://github.com/apache/camel/pull/4876#discussion_r556506921



##########
File path: components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/stream/GoogleCalendarStreamConsumer.java
##########
@@ -59,19 +70,21 @@ public GoogleCalendarStreamEndpoint getEndpoint() {
 
     @Override
     protected int poll() throws Exception {
-        com.google.api.services.calendar.Calendar.Events.List request
-                = getClient().events().list(getConfiguration().getCalendarId()).setOrderBy("updated");
+        Calendar.Events.List request = getClient().events().list(getConfiguration().getCalendarId());
         if (ObjectHelper.isNotEmpty(getConfiguration().getQuery())) {

Review comment:
       Then instead of doing this check here in the poll, check if syncFlow is true and query is set in the endpoint consumer creation and fail there. Don't use Precondition, is just another class that will change in the future and we'll have to align, you just need to check a boolean value.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] jmerljak commented on a change in pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox
In reply to this post by GitBox

jmerljak commented on a change in pull request #4876:
URL: https://github.com/apache/camel/pull/4876#discussion_r556522025



##########
File path: components/camel-google-calendar/src/main/java/org/apache/camel/component/google/calendar/stream/GoogleCalendarStreamConsumer.java
##########
@@ -59,19 +70,21 @@ public GoogleCalendarStreamEndpoint getEndpoint() {
 
     @Override
     protected int poll() throws Exception {
-        com.google.api.services.calendar.Calendar.Events.List request
-                = getClient().events().list(getConfiguration().getCalendarId()).setOrderBy("updated");
+        Calendar.Events.List request = getClient().events().list(getConfiguration().getCalendarId());
         if (ObjectHelper.isNotEmpty(getConfiguration().getQuery())) {

Review comment:
       Done.




----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] oscerd closed pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox
In reply to this post by GitBox

oscerd closed pull request #4876:
URL: https://github.com/apache/camel/pull/4876


   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] oscerd commented on pull request #4876: [CAMEL-15961] Google Calendar synchronization flow

GitBox
In reply to this post by GitBox

oscerd commented on pull request #4876:
URL: https://github.com/apache/camel/pull/4876#issuecomment-759988409


   Merged


----------------------------------------------------------------
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:
[hidden email]