[GitHub] [incubator-gobblin] arjun4084346 commented on a change in pull request #3078: [GOBBLIN-1233] Add case-aware support in WhitelistBlacklist and other small fixes

[GitBox <gi...@apache.org>]

arjun4084346 commented on a change in pull request #3078:
URL: https://github.com/apache/incubator-gobblin/pull/3078#discussion_r470398611



##########
File path: gobblin-utility/src/main/java/org/apache/gobblin/data/management/copy/hive/WhitelistBlacklist.java
##########
@@ -53,23 +53,36 @@
 
   public static final String WHITELIST = "whitelist";
   public static final String BLACKLIST = "blacklist";
+  public static final String IGNORE_CASE = "whitelistBlacklist.ignoreCase";

Review comment:
       Please update the javadoc explaining IGNORE_CASE

##########
File path: gobblin-utility/src/main/java/org/apache/gobblin/data/management/copy/hive/WhitelistBlacklist.java
##########
@@ -53,23 +53,36 @@
 
   public static final String WHITELIST = "whitelist";
   public static final String BLACKLIST = "blacklist";
+  public static final String IGNORE_CASE = "whitelistBlacklist.ignoreCase";
 
   private static final Pattern ALL_TABLES = Pattern.compile(".*");
 
   private final SetMultimap<Pattern, Pattern> whitelistMultimap;
   private final SetMultimap<Pattern, Pattern> blacklistMultimap;
+  private final boolean ignoreCase;
 
   public WhitelistBlacklist(Config config) throws IOException {
-    this(config.hasPath(WHITELIST) ? config.getString(WHITELIST).toLowerCase() : "",
-        config.hasPath(BLACKLIST) ? config.getString(BLACKLIST).toLowerCase() : "");
+    this(config.hasPath(WHITELIST) ? config.getString(WHITELIST) : "",
+        config.hasPath(BLACKLIST) ? config.getString(BLACKLIST) : "",
+        !config.hasPath(IGNORE_CASE) || config.getBoolean(IGNORE_CASE));
   }
 
   public WhitelistBlacklist(String whitelist, String blacklist) throws IOException {
+    this(whitelist, blacklist, true);
+  }
+
+  public WhitelistBlacklist(String whitelist, String blacklist, boolean ignoreCase) throws IOException {
     this.whitelistMultimap = HashMultimap.create();
     this.blacklistMultimap = HashMultimap.create();
 
-    populateMultimap(this.whitelistMultimap, whitelist.toLowerCase());
-    populateMultimap(this.blacklistMultimap, blacklist.toLowerCase());
+    this.ignoreCase = ignoreCase;

Review comment:
       Nit a line break could be after setting ignoreCase and not before it.

##########
File path: gobblin-core/src/main/java/org/apache/gobblin/writer/partitioner/TimeBasedWriterPartitioner.java
##########
@@ -74,6 +76,7 @@
   private final String writerPartitionSuffix;
   private final DatePartitionType granularity;
   private final DateTimeZone timeZone;
+  @Getter

Review comment:
       required?




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