You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by GitBox <gi...@apache.org> on 2020/03/05 18:48:46 UTC

[GitHub] [incubator-gobblin] arekusuri opened a new pull request #2910: Add typed config in salesforce

arekusuri opened a new pull request #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910
 
 
   Dear Gobblin maintainers,
   
   Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
   
   
   ### JIRA
   
   ### Description
   replace `properties.get()` with typed class field `config.bulkApiUseQueryAll`.
   It would be neater/readable and any typo can be find out at compiling time.
   
   >  - workUnitState.getPropAsBoolean(BULK_API_USE_QUERY_ALL, DEFAULT_BULK_API_USE_QUERY_ALL);
   > + this.bulkApiUseQueryAll = conf.bulkApiUseQueryAll;
   
   ### Tests
   https://ltx1-holdemaz05.grid.linkedin.com:8443/executor?execid=23983645&job=salesforce_task_full&attempt=0
   
   
   
   

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] autumnust commented on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
autumnust commented on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-595425499
 
 
   Please go through https://gobblin.readthedocs.io/en/latest/developer-guide/CodingStyle/ for style guidance, also all license header for newly created files are missing. 
   
   Also, an Linkedin-specific test link is not useful for other folks. You may consider unit tests for `ConstraintUtil` to maintain code-coverage. 

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arekusuri edited a comment on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri edited a comment on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-604003936
 
 
   > @autumnust, @arekusuri , If this style make sense and dont see any possible problem, I think we should put some of the core classes in gobblin.config package and start using this style everywhere. looking at config improvement myself, I like this style.
   > @arekusuri , btw, this could be very nice feature set on top of [typesafe](https://github.com/lightbend/config) or [tscfg](https://github.com/carueda/tscfg) itself. Thanks for the improvements. let me know if you need any help to extend this to all gobblin config.
   
   Thanks for your reviewing. Here is the [design doc](https://docs.google.com/document/d/1G4RgZTQFqXSLUAVlLQrA94Ges8Q2SUciNIUmMwhsNt4/edit#)

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arekusuri commented on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-595499648
 
 
   > Alex. Thanks for taking the action to improve the config key story. Is there a grand plan for the gobblin configuration system? Or you're just trying to improve life of gobblin dev to work with salesforce configurations, for gobblin users, they have to deal with strings.
   
   It is flexible enough for both cases, because it takes a property dataset to fill into a config class instance. There is no Gobblin framework structure change.
   If Gobblin core doesn't work to adopt it yet, gobblin core can continue use the old way.
   Runtime feature are all completed, includes "string". The type is decided by config field's type.
   >   @Key("salesforce.test.jobIdAndBulkIdPair")@Default("1:11")@StringRegex("[0-9]+:[0-9]+")
     public String jobIdAndBulkIdPair;
   In this code, `jobIdAndBulkIdPair` will be filled in with string value. And the value set up in INI file must satisfy the restrain `regex [0-9]+:[0-9]+`
   Future plan is to valid whether INI value satisfy the regex at compiling time by implement an annotation processor.
   

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arekusuri edited a comment on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri edited a comment on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-595499648
 
 
   > Alex. Thanks for taking the action to improve the config key story. Is there a grand plan for the gobblin configuration system? Or you're just trying to improve life of gobblin dev to work with salesforce configurations, for gobblin users, they have to deal with strings.
   
   It is flexible enough for both cases, because it takes a property dataset to fill into a config class instance. There is no Gobblin framework structure change.
   If Gobblin core doesn't want to adopt it yet, Gobblin core can continue use the old way.
   Runtime features are all completed, includes "string". The type is decided by config field's type.
   >   @Key("salesforce.test.jobIdAndBulkIdPair")@Default("1:11")@StringRegex("[0-9]+:[0-9]+")
     public String jobIdAndBulkIdPair;
   
   In this code, `jobIdAndBulkIdPair` will be filled in with string value. And the value set up in INI file must satisfy the restrain `regex [0-9]+:[0-9]+`
   Future plan is to valid whether INI value satisfy the regex at compiling time by implement an annotation processor.
   

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arekusuri edited a comment on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri edited a comment on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-595499648
 
 
   > Alex. Thanks for taking the action to improve the config key story. Is there a grand plan for the gobblin configuration system? Or you're just trying to improve life of gobblin dev to work with salesforce configurations, for gobblin users, they have to deal with strings.
   
   It is flexible enough for both cases, because it takes a property dataset to fill into a config class instance. There is no Gobblin framework structure change.
   If Gobblin core doesn't work to adopt it yet, gobblin core can continue use the old way.
   Runtime feature are all completed, includes "string". The type is decided by config field's type.
   >   @Key("salesforce.test.jobIdAndBulkIdPair")@Default("1:11")@StringRegex("[0-9]+:[0-9]+")
     public String jobIdAndBulkIdPair;
   
   In this code, `jobIdAndBulkIdPair` will be filled in with string value. And the value set up in INI file must satisfy the restrain `regex [0-9]+:[0-9]+`
   Future plan is to valid whether INI value satisfy the regex at compiling time by implement an annotation processor.
   

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arekusuri commented on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-604003936
 
 
   > @autumnust, @arekusuri , If this style make sense and dont see any possible problem, I think we should put some of the core classes in gobblin.config package and start using this style everywhere. looking at config improvement myself, I like this style.
   > @arekusuri , btw, this could be very nice feature set on top of [typesafe](https://github.com/lightbend/config) or [tscfg](https://github.com/carueda/tscfg) itself. Thanks for the improvements. let me know if you need any help to extend this to all gobblin config.
   
   Thanks for your reviewing. Here is the [design doc](https://docs.google.com/document/d/1ZqRN4PHq2LXFMfMwpbe9UHfgOO-xIRUKdwzZ24b-F00/edit#)

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] jhsenjaliya commented on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
jhsenjaliya commented on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-602287114
 
 
   @autumnust, @arekusuri , If this style make sense and dont see any possible problem,  I think we should put some of the core classes in gobblin.config package and start using this style everywhere. looking at config improvement myself, I like this style.
   @arekusuri , btw, this could be very nice feature set on top of [typesafe](https://github.com/lightbend/config) or [tscfg](https://github.com/carueda/tscfg) itself. Thanks for the improvements. let me know if you need any help to extend this to all gobblin config.

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] zxcware commented on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
zxcware commented on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-595489394
 
 
   Alex. Thanks for taking the action to improve the config key story. Is there a grand plan for the gobblin configuration system? Or you're just trying to improve life of gobblin dev to work with salesforce configurations, for gobblin users, they have to deal with strings.

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] arekusuri commented on issue #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on issue #2910: Add typed config in salesforce
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-595495275
 
 
   > Please go through https://gobblin.readthedocs.io/en/latest/developer-guide/CodingStyle/ for style guidance, also all license header for newly created files are missing.
   > 
   > Also, an Linkedin-specific test link is not useful for other folks. You may consider unit tests for `ConstraintUtil` to maintain code-coverage.
   
   Thanks for your comment. This is a very early version, I am not sure how far it could go.  My original idea was to create a separate lib project and simply add a library into gobblin project.

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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-commenter edited a comment on pull request #2910: [GOBBLIN-1179] Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-638520374


   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=h1) Report
   > Merging [#2910](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/469a56d9161c404c6dc73e8fe927fe2bb8922ddd&el=desc) will **decrease** coverage by `0.07%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2910      +/-   ##
   ============================================
   - Coverage     45.72%   45.65%   -0.08%     
   + Complexity     9295     9292       -3     
   ============================================
     Files          1951     1955       +4     
     Lines         74255    74366     +111     
     Branches       8218     8246      +28     
   ============================================
   - Hits          33953    33950       -3     
   - Misses        37129    37243     +114     
     Partials       3173     3173              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...che/gobblin/salesforce/QueryBasedSourceConfig.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NhbGVzZm9yY2UvUXVlcnlCYXNlZFNvdXJjZUNvbmZpZy5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...apache/gobblin/salesforce/SalesforceExtractor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NhbGVzZm9yY2UvU2FsZXNmb3JjZUV4dHJhY3Rvci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...n/java/org/apache/gobblin/salesforce/SfConfig.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NhbGVzZm9yY2UvU2ZDb25maWcuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...org/apache/gobblin/typedconfig/ConstraintUtil.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3R5cGVkY29uZmlnL0NvbnN0cmFpbnRVdGlsLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...va/org/apache/gobblin/typedconfig/TypedConfig.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3R5cGVkY29uZmlnL1R5cGVkQ29uZmlnLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0.00%> (-7.15%)` | `3.00% <0.00%> (ø%)` | |
   | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `34.92% <0.00%> (-5.56%)` | `14.00% <0.00%> (-2.00%)` | |
   | [.../org/apache/gobblin/metrics/RootMetricContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9Sb290TWV0cmljQ29udGV4dC5qYXZh) | `78.12% <0.00%> (-1.57%)` | `15.00% <0.00%> (-1.00%)` | |
   | ... and [5 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=footer). Last update [469a56d...5f662b6](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r428382264



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,80 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+public class ConstraintUtil {
+  private ConstraintUtil() {
+  }
+
+  public static Object constraint(Field field, Object value, Object defaultValue) {
+    if (value == null) {
+      return defaultValue;
+    }
+    Class type = field.getType();
+    if (type == int.class) {

Review comment:
       create separate function for each type of data.




----------------------------------------------------------------
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] [incubator-gobblin] htran1 commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
htran1 commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r430757448



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+/**
+ * Util class for handling constrain annotation
+ * supports: IntRange, LongRange, EnumOption, StringRegex
+ */
+@Slf4j
+public class ConstraintUtil {
+  private ConstraintUtil() {
+  }
+
+  public static Object constraint(Field field, Object value, Object defaultValue) {
+    if (value == null) {
+      return defaultValue;
+    }
+    Class type = field.getType();
+    if (type.isEnum()) {
+      return getEnumValue(field, value, defaultValue);
+    }
+    switch (type.getName()) {
+      case "int": case "java.lang.Integer":
+        return getIntValue(field, value, defaultValue);
+      case "long": case "java.lang.Long":
+        return getLongValue(field, value, defaultValue);
+      case "java.lang.String":
+        return getStringValue(field, value, defaultValue);
+      case "boolean": case "java.lang.Boolean":
+        return getBooleanValue(field, value, defaultValue);
+      case "java.util.Date":
+        return getDateValue(field, value, defaultValue);
+      default:
+          throw new RuntimeException("not supported the return type: " + type.getName());
+    }
+  }
+
+  static private Object getIntValue(Field field, Object value, Object defaultValue) {
+    IntRange intRange = field.getAnnotation(IntRange.class);
+    if (intRange == null) {
+      return value;
+    }
+    int[] range = intRange.value();
+    int intValue = Integer.parseInt(value.toString());
+    if (range.length != 2 || (range.length == 2 && intValue >= range[0] && intValue <= range[1])) {
+      return value;
+    } else {
+      return defaultValue;
+    }
+  }
+  static private Object getLongValue(Field field, Object value, Object defaultValue) {
+    LongRange longRange = field.getAnnotation(LongRange.class);
+    long[] range = longRange.value();
+    if (range == null) {
+      return value;
+    }
+    long longValue = Long.parseLong(value.toString());
+    if (range.length != 2 || (range.length == 2 && longValue > range[0] && longValue < range[1])) {

Review comment:
       We should raise an error instead of using the default if an out of range value is provided. This is to avoid confusion where the user thinks that the execution used the config they provide.

##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+/**
+ * Util class for handling constrain annotation

Review comment:
       constrain -> constraint




----------------------------------------------------------------
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] [incubator-gobblin] codecov-commenter commented on pull request #2910: [GOBBLIN-1179] Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#issuecomment-638520374


   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=h1) Report
   > Merging [#2910](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/469a56d9161c404c6dc73e8fe927fe2bb8922ddd&el=desc) will **decrease** coverage by `0.07%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2910      +/-   ##
   ============================================
   - Coverage     45.72%   45.65%   -0.08%     
   + Complexity     9295     9292       -3     
   ============================================
     Files          1951     1955       +4     
     Lines         74255    74366     +111     
     Branches       8218     8246      +28     
   ============================================
   - Hits          33953    33950       -3     
   - Misses        37129    37243     +114     
     Partials       3173     3173              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...che/gobblin/salesforce/QueryBasedSourceConfig.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NhbGVzZm9yY2UvUXVlcnlCYXNlZFNvdXJjZUNvbmZpZy5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...apache/gobblin/salesforce/SalesforceExtractor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NhbGVzZm9yY2UvU2FsZXNmb3JjZUV4dHJhY3Rvci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...n/java/org/apache/gobblin/salesforce/SfConfig.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NhbGVzZm9yY2UvU2ZDb25maWcuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...org/apache/gobblin/typedconfig/ConstraintUtil.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3R5cGVkY29uZmlnL0NvbnN0cmFpbnRVdGlsLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...va/org/apache/gobblin/typedconfig/TypedConfig.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3R5cGVkY29uZmlnL1R5cGVkQ29uZmlnLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...lin/util/filesystem/FileSystemInstrumentation.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsZXN5c3RlbS9GaWxlU3lzdGVtSW5zdHJ1bWVudGF0aW9uLmphdmE=) | `85.71% <0.00%> (-7.15%)` | `3.00% <0.00%> (ø%)` | |
   | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `34.92% <0.00%> (-5.56%)` | `14.00% <0.00%> (-2.00%)` | |
   | [.../org/apache/gobblin/metrics/RootMetricContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9Sb290TWV0cmljQ29udGV4dC5qYXZh) | `78.12% <0.00%> (-1.57%)` | `15.00% <0.00%> (-1.00%)` | |
   | ... and [5 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2910/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=footer). Last update [469a56d...5f662b6](https://codecov.io/gh/apache/incubator-gobblin/pull/2910?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r426357233



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/TypedConfig.java
##########
@@ -0,0 +1,126 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.Properties;
+import lombok.SneakyThrows;
+import org.apache.commons.beanutils.BeanUtilsBean;
+import org.apache.commons.beanutils.ConvertUtilsBean;
+
+
+public class TypedConfig {
+
+  public TypedConfig(Properties prop) {
+    if (prop != null) {
+      fulFillFields(this.getClass(), prop);
+      afterFulfill();
+    }
+  }
+
+  /**
+   * subclass could do something after fulfill by override afterFulfill
+   */
+  protected void afterFulfill() {
+  }
+
+  @SneakyThrows
+  private void fulFillFields(Class<? extends TypedConfig> clazz, Properties prop) {
+    if (!clazz.equals(TypedConfig.class)) {
+      this.fulFillFields((Class<? extends TypedConfig>) clazz.getSuperclass(), prop);
+    }
+    for (Field field : clazz.getDeclaredFields()) {
+      if (field.getAnnotations().length == 0) {
+        continue;
+      }
+      Object defaultValue = pickupDefaultValue(field); // get default value which is in config class
+      Object configValue = pickupValueByKey(field, prop); // get ini file config value by key
+      if (configValue == null) {
+        configValue = pickupValueByAlias(field, prop); // by alias (2nd key)
+      }
+      if (configValue == null) {
+        configValue = defaultValue;
+      }
+      if (configValue == null) {
+        continue;
+      }
+      configValue = ConstraintUtil.constraint(field, configValue, defaultValue);
+      field.set(this, convert(configValue, field.getType()));
+    }
+  }
+
+  private Object pickupDefaultValue(Field field) {
+    Default defaultAnn = field.getAnnotation(Default.class);
+    if (defaultAnn == null) {
+      return null;
+    }
+    return defaultAnn.value(); // the value was put in source code instead of ini file
+  }
+
+  private Object pickupValueByAlias(Field field, Properties prop) {
+    Alias alias = field.getAnnotation(Alias.class);
+    if (alias == null) {
+      return null;
+    }
+    return prop.get(alias.value()); // get ini config value by alias(2nd key)
+  }
+
+  private Object pickupValueByKey(Field field,  Properties prop) {
+    Key key = field.getAnnotation(Key.class);
+    if (key == null) {
+      return null;
+    }
+    return prop.get(key.value()); // get ini config value by key
+  }
+
+  private Object convert(Object value, Class targetClazz) {
+    if (value == null) {
+      return null;
+    }
+    BeanUtilsBean beanUtilsBean = new BeanUtilsBean(new ConvertUtilsBean() {
+      @SneakyThrows
+      @Override
+      public Object convert(Object value, Class clazz) {
+        if (clazz.isEnum()) {
+          return Enum.valueOf(clazz, (String) value);
+        } else if (clazz == Date.class) {
+          String dateStr = ((String) value).replaceAll("-| |:", "");
+          dateStr = String.format("%-14s", dateStr).replaceAll(" ", "0");
+          Date date = new SimpleDateFormat("yyyyMMddHHmmss").parse(dateStr);
+          return date;
+        } else {
+          return super.convert(value, clazz);
+        }
+      }
+    });
+    return beanUtilsBean.getConvertUtils().convert(value, targetClazz);
+  }
+
+  /**
+   * subclass could do something before toProp by overriding
+   */
+  protected void beforeToProp() {

Review comment:
       removed.




----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r426356757



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/TypedConfig.java
##########
@@ -0,0 +1,126 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.Properties;
+import lombok.SneakyThrows;
+import org.apache.commons.beanutils.BeanUtilsBean;
+import org.apache.commons.beanutils.ConvertUtilsBean;
+
+
+public class TypedConfig {
+
+  public TypedConfig(Properties prop) {
+    if (prop != null) {
+      fulFillFields(this.getClass(), prop);
+      afterFulfill();
+    }
+  }
+
+  /**
+   * subclass could do something after fulfill by override afterFulfill
+   */
+  protected void afterFulfill() {

Review comment:
       removed.




----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r426230577



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/Alias.java
##########
@@ -0,0 +1,13 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+
+@Target({ElementType.FIELD, ElementType.TYPE})
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Alias {

Review comment:
       One. Do we want multiple?




----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r428382264



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,80 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+public class ConstraintUtil {
+  private ConstraintUtil() {
+  }
+
+  public static Object constraint(Field field, Object value, Object defaultValue) {
+    if (value == null) {
+      return defaultValue;
+    }
+    Class type = field.getType();
+    if (type == int.class) {

Review comment:
       create separate function for every type of data.




----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r426354283



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/TypedConfig.java
##########
@@ -0,0 +1,126 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.Properties;
+import lombok.SneakyThrows;
+import org.apache.commons.beanutils.BeanUtilsBean;
+import org.apache.commons.beanutils.ConvertUtilsBean;
+
+
+public class TypedConfig {
+
+  public TypedConfig(Properties prop) {
+    if (prop != null) {
+      fulFillFields(this.getClass(), prop);

Review comment:
       good catch!




----------------------------------------------------------------
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] [incubator-gobblin] zxcware commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
zxcware commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r428329643



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,80 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+public class ConstraintUtil {
+  private ConstraintUtil() {
+  }
+
+  public static Object constraint(Field field, Object value, Object defaultValue) {
+    if (value == null) {
+      return defaultValue;
+    }
+    Class type = field.getType();
+    if (type == int.class) {

Review comment:
       I mean something like this https://github.com/apache/incubator-gobblin/blob/master/gobblin-modules/gobblin-parquet/src/main/java/org/apache/gobblin/converter/parquet/JsonElementConversionFactory.java.




----------------------------------------------------------------
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] [incubator-gobblin] zxcware commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
zxcware commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r429358450



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/Alias.java
##########
@@ -0,0 +1,13 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+
+@Target({ElementType.FIELD, ElementType.TYPE})
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Alias {

Review comment:
       one is better. We should control it.




----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r430857677



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+/**
+ * Util class for handling constrain annotation
+ * supports: IntRange, LongRange, EnumOption, StringRegex
+ */
+@Slf4j
+public class ConstraintUtil {
+  private ConstraintUtil() {
+  }
+
+  public static Object constraint(Field field, Object value, Object defaultValue) {
+    if (value == null) {
+      return defaultValue;
+    }
+    Class type = field.getType();
+    if (type.isEnum()) {
+      return getEnumValue(field, value, defaultValue);
+    }
+    switch (type.getName()) {
+      case "int": case "java.lang.Integer":
+        return getIntValue(field, value, defaultValue);
+      case "long": case "java.lang.Long":
+        return getLongValue(field, value, defaultValue);
+      case "java.lang.String":
+        return getStringValue(field, value, defaultValue);
+      case "boolean": case "java.lang.Boolean":
+        return getBooleanValue(field, value, defaultValue);
+      case "java.util.Date":
+        return getDateValue(field, value, defaultValue);
+      default:
+          throw new RuntimeException("not supported the return type: " + type.getName());
+    }
+  }
+
+  static private Object getIntValue(Field field, Object value, Object defaultValue) {
+    IntRange intRange = field.getAnnotation(IntRange.class);
+    if (intRange == null) {
+      return value;
+    }
+    int[] range = intRange.value();
+    int intValue = Integer.parseInt(value.toString());
+    if (range.length != 2 || (range.length == 2 && intValue >= range[0] && intValue <= range[1])) {
+      return value;
+    } else {
+      return defaultValue;
+    }
+  }
+  static private Object getLongValue(Field field, Object value, Object defaultValue) {
+    LongRange longRange = field.getAnnotation(LongRange.class);
+    long[] range = longRange.value();
+    if (range == null) {
+      return value;
+    }
+    long longValue = Long.parseLong(value.toString());
+    if (range.length != 2 || (range.length == 2 && longValue > range[0] && longValue < range[1])) {

Review comment:
       Thanks! it makes sense.

##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+/**
+ * Util class for handling constrain annotation

Review comment:
       good catch! thanks!




----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r428382111



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/Alias.java
##########
@@ -0,0 +1,13 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+
+@Target({ElementType.FIELD, ElementType.TYPE})
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Alias {

Review comment:
       changed to multiple.




----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r426356133



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,80 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+public class ConstraintUtil {

Review comment:
       Added. Thanks!




----------------------------------------------------------------
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] [incubator-gobblin] arekusuri commented on a change in pull request #2910: Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910#discussion_r426354195



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/typedconfig/ConstraintUtil.java
##########
@@ -0,0 +1,80 @@
+package org.apache.gobblin.typedconfig;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import org.apache.gobblin.typedconfig.compiletime.EnumOptions;
+import org.apache.gobblin.typedconfig.compiletime.IntRange;
+import org.apache.gobblin.typedconfig.compiletime.LongRange;
+import org.apache.gobblin.typedconfig.compiletime.StringRegex;
+
+
+public class ConstraintUtil {
+  private ConstraintUtil() {
+  }
+
+  public static Object constraint(Field field, Object value, Object defaultValue) {
+    if (value == null) {
+      return defaultValue;
+    }
+    Class type = field.getType();
+    if (type == int.class) {

Review comment:
       Better keep using if-else. I think `switch` is like for the case there is very little code between  switch-cases. https://docs.oracle.com/javase/8/docs/technotes/guides/language/strings-switch.html
   




----------------------------------------------------------------
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] [incubator-gobblin] asfgit closed pull request #2910: [GOBBLIN-1179] Add typed config in salesforce

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2910:
URL: https://github.com/apache/incubator-gobblin/pull/2910


   


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