[GitHub] [gobblin] aplex commented on a change in pull request #3361: [GOBBLIN-1513] fixed the construction of regex filters

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

aplex commented on a change in pull request #3361:
URL: https://github.com/apache/gobblin/pull/3361#discussion_r686139470



##########
File path: gobblin-utility/src/main/java/org/apache/gobblin/util/filters/RegexPathFilter.java
##########
@@ -34,6 +35,11 @@
 
   private final Pattern regex;
   private final boolean include;
+  public static final String REGEX = "path.filter.regex";

Review comment:
       So, is this path filter only used to filter source datasets?
   I can see that we specify the class using gobblin.dataset.path.filter.class . It could make sense to have the same "gobblin.dataset" prefix for this setting for consistency.
   
   Also, it could be better to keep those config settings together, so that they would be easily discovered.
   
   Usually, they land up in ConfigKeys class, but I can see that bunch of keys are defined in DatasetUtils. You can try to figure out if there is any reason to keep them there.




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