[GitHub] [drill] luocooong commented on a change in pull request #2225: DRILL-7922: Add import profile function

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

luocooong commented on a change in pull request #2225:
URL: https://github.com/apache/drill/pull/2225#discussion_r638889212



##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
##########
@@ -83,6 +86,8 @@
   @Inject
   HttpServletRequest request;
 
+  static String importedProfile = null;

Review comment:
       We are using a variable to store the profile and avoid loading the profile from disk, that's good. But how to do that if multiple users use the function at the same time...

##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
##########
@@ -395,6 +406,22 @@ public Viewable getProfile(@PathParam("queryid") String queryId){
     }
   }
 
+  @POST
+  @Path("/profiles/view")
+  @Consumes(MediaType.MULTIPART_FORM_DATA)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable viewProfile(@FormDataParam("profileFile") String content){

Review comment:
       It is possible to rename the "profileFile" to "profile_object" or "profile_data" ... ?

##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
##########
@@ -395,6 +406,22 @@ public Viewable getProfile(@PathParam("queryid") String queryId){
     }
   }
 
+  @POST
+  @Path("/profiles/view")

Review comment:
       Good naming : `view`, not the `import`. Could you please rename all names related to the "import" to "view" ?

##########
File path: exec/java-exec/src/main/resources/rest/profile/list.ftl
##########
@@ -84,6 +84,13 @@
         $("#queryCancelModal").modal("show");
     }
 
+    function uploadProfile() {

Review comment:
       Add a comment above the function.

##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
##########
@@ -395,6 +406,22 @@ public Viewable getProfile(@PathParam("queryid") String queryId){
     }
   }
 
+  @POST
+  @Path("/profiles/view")
+  @Consumes(MediaType.MULTIPART_FORM_DATA)
+  @Produces(MediaType.TEXT_HTML)
+  public Viewable viewProfile(@FormDataParam("profileFile") String content){
+    try {
+      importedProfile = content;
+      QueryProfile profile = work.getContext().getProfileStoreContext().getProfileStoreConfig().getSerializer().deserialize(content.getBytes());
+      ProfileWrapper wrapper = new ProfileWrapper(profile, work.getContext().getConfig(), request);
+      return ViewableWithPermissions.create(authEnabled.get(), "/rest/profile/profile.ftl", sc, wrapper);
+    } catch (Exception | Error e) {
+      logger.error("Exception was thrown when parsing profile {}:\n{}", content, e);

Review comment:
       Add a space between the "profile {}" and ":".




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