-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
forceFlag was never used to recreate client with the new access token #76
forceFlag was never used to recreate client with the new access token #76
Conversation
@@ -95,7 +95,7 @@ protected int poll() throws Exception { | |||
postProcess(fileModel, file); | |||
} | |||
} catch (GoogleJsonResponseException e) { | |||
LOG.error("Google Drive Consume error:", e); | |||
LOG.info(" >> GoogleJsonResponseException occurred"); | |||
service = this.endpoint.getClient(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not good to set an exception in an info log. Better to change it into a warning. Also, when you get this as user, you want to have more info. I would suggest something like this (pseudocode):
if(connectionAttempts < 60){
LOG.warn("Could not connect to Google Drive directory + directory. Try again after x seconds (attempt " + connectionAttempts + " of 60)");
connectionAttempts += 1;
service = this.endpoint.getClient(true);
}else{
LOG.error("GoogleJsonResponseException occurred. Connect to Google Drive directory + directory failed:", e);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x = the poll interval. The user can change the poll interval to try it for a longer period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes done!
x = should be the delay parameter set by the user
@@ -72,7 +76,8 @@ public boolean isSingleton() { | |||
} | |||
|
|||
Drive getClient(boolean forceFlag) { | |||
if (client == null) { | |||
if (client == null || forceFlag) { | |||
LOG.info(" >> Get client"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line doesn't say that much. Either leave it away or provide more info what's going on:
LOG.info("Get GoogleDrive client with clientid=" + );
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this log
@@ -22,6 +24,8 @@ | |||
@SuppressWarnings("PackageAccessibility") | |||
public class GoogleDriveEndpoint extends ProcessorEndpoint { | |||
|
|||
private static final Logger LOG = LoggerFactory.getLogger(GoogleDriveEndpoint.class); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a more generic (and I think recommended way) to get the logger:
protected Logger log = LoggerFactory.getLogger(getClass());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleted line. don't need anymore
No description provided.