[GitHub] [gobblin] phet commented on a diff in pull request #3773: Follow-up changes based on #3768

["phet (via GitHub)" <gi...@apache.org>]

phet commented on code in PR #3773:
URL: https://github.com/apache/gobblin/pull/3773#discussion_r1333962323


##########
gobblin-salesforce/src/main/java/org/apache/gobblin/salesforce/SalesforceSource.java:
##########
@@ -462,9 +459,9 @@ private static Set<SourceEntity> getSourceEntities(String response) {
   }
 
   protected SalesforceConnector getConnector(State state) {
-    if (this.salesforceConnector == null) {
-      this.salesforceConnector = new SalesforceConnector(state);
+    if (salesforceConnector == null) {
+      salesforceConnector = new SalesforceConnector(state);

Review Comment:
   this is not thread-safe... should it be `synchronized`?  (if there's any need to explicitly close the connector, there's a potential resource leak.)



##########
gobblin-salesforce/src/main/java/org/apache/gobblin/salesforce/SalesforceSource.java:
##########
@@ -133,11 +130,11 @@ protected void addLineageSourceInfo(SourceState sourceState, SourceEntity entity
   }
   @Override
   protected List<WorkUnit> generateWorkUnits(SourceEntity sourceEntity, SourceState state, long previousWatermark) {
-    SalesforceConnector connector = getConnector(state);
 
     SfConfig sfConfig = new SfConfig(state.getProperties());
-    if (salesforceHistogramService == null) {
-      salesforceHistogramService = new SalesforceHistogramService(sfConfig, connector);
+    if (histogramService == null) {
+      salesforceConnector = getConnector(state);
+      histogramService = new RecordModTimeHistogramService(sfConfig, getConnector(state));

Review Comment:
   maybe add comment on why another `getConnector(state)`, rather than reusing
   
   edit: reading on I see it doesn't create a new connector... just looks that way



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

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

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