Dmitry Lychagin has posted comments on this change. ( 
https://asterix-gerrit.ics.uci.edu/3386 )

Change subject: [ASTERIXDB-2597] Load UDFs via HTTP
......................................................................


Patch Set 10: Code-Review+1

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/3386/9/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/UdfApiServlet.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/UdfApiServlet.java:

https://asterix-gerrit.ics.uci.edu/#/c/3386/9/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/UdfApiServlet.java@96
PS9, Line 96:             ByteBuffer content = reqContent.nioBuffer();
> It's the classloader of ExternalLibraryUtils.
so the deployed code will have access to all Hyracks/Asterix classes. We'll 
need to fix it at some point.


https://asterix-gerrit.ics.uci.edu/#/c/3386/10/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/UdfApiServlet.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/UdfApiServlet.java:

https://asterix-gerrit.ics.uci.edu/#/c/3386/10/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/UdfApiServlet.java@88
PS10, Line 88:             udf = File.createTempFile(
are you sure this does the right thing? The temp directory should be the third 
argument to createTempFile() and I think it should already exist. So I'd write 
this a something like:
File.createTempFile(resourceName,  ".zip", new File(                  
appCtx.getServiceContext().getServerCtx().getBaseDir(),                         
   UDF_TMP_DIR_PREFIX));


https://asterix-gerrit.ics.uci.edu/#/c/3386/10/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/bootstrap/TestNodeController.java
File 
asterixdb/asterix-app/src/test/java/org/apache/asterix/app/bootstrap/TestNodeController.java:

https://asterix-gerrit.ics.uci.edu/#/c/3386/10/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/bootstrap/TestNodeController.java@167
PS10, Line 167:         //        ExternalUDFLibrarian.removeLibraryDir();
minor. remove commented out code


https://asterix-gerrit.ics.uci.edu/#/c/3386/10/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/bootstrap/TestNodeController.java@167
PS10, Line 167:         //        ExternalUDFLibrarian.removeLibraryDir();
minor. remove commented out code.



--
To view, visit https://asterix-gerrit.ics.uci.edu/3386
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6be9fef54c010bdb32f5c78af9b973f9843f442f
Gerrit-Change-Number: 3386
Gerrit-PatchSet: 10
Gerrit-Owner: Ian Maxon <[email protected]>
Gerrit-Reviewer: Anon. E. Moose (1000171)
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Comment-Date: Tue, 25 Jun 2019 23:31:39 +0000
Gerrit-HasComments: Yes

Reply via email to