[GitHub] [drill] ihuzenko commented on a change in pull request #1712: DRILL-7079: Drill can't query views from the S3 storage when plain authentication is enabled

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

ihuzenko commented on a change in pull request #1712: DRILL-7079: Drill can't query views from the S3 storage when plain authentication is enabled
URL: https://github.com/apache/drill/pull/1712#discussion_r267835326
 
 

 ##########
 File path: exec/java-exec/src/main/java/org/apache/drill/exec/dotdrill/DotDrillFile.java
 ##########
 @@ -55,6 +56,13 @@ public DotDrillType getType(){
    * @return Return owner of the file in underlying file system.
    */
   public String getOwner() {
+    if (type == DotDrillType.VIEW && status.getOwner().isEmpty()) {
 
 Review comment:
   If owner is not always empty for .view.drill files on S3, maybe it makes sense to inverse the check a little bit, like: 
   
   ```java
     public String getOwner() {
       String owner = status.getOwner();
       if (owner.isEmpty() && type == DotDrillType.VIEW) {
         // Drill view S3AFileStatus is not populated with owner (it has default value of "").
         // This empty String causes IllegalArgumentException to be thrown (if impersonation is enabled) in
         // SchemaTreeProvider#createRootSchema(String, SchemaConfigInfoProvider). To work-around the issue
         // we can return current user as if they were the owner of the file (since they have access to it).
         owner = ImpersonationUtil.getProcessUserName();
       }
       return owner;
     }
   ```
   Also what if owner is empty but file type is not ```DotDrillType.VIEW``` ? Should we throw exception in such case to detect potential future problems early in such case ?  
   
   

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