[Part 6]: Blob Store abstractions and a classloader factory that uses the blob store #155
Conversation
…use it Two features are added through this change. 1). Firstly, I'm introducing a new blob store abstraction and a few implementations that can help download the blob from various stores such as s3, artifactory, etc... 2). A new class loader implementation has also been added that can use the blob store to download the requested artifacts from s3 or artifactory or elsewhere and create a class loader object that can run the user's application code.
sundargates
requested review from
codyrioux,
nickmahilani,
jeffchao,
piygoyal,
calvin681,
Andyz26 and
hmitnflx
as code owners
| @@ -0,0 +1,13 @@ | |||
| # EditorConfig helps developers define and maintain consistent | |||
There was a problem hiding this comment.
Great call. I just installed the plugin for my editor.
mantis-server/mantis-server-worker/src/main/java/io/mantisrx/server/worker/BlobStore.java
Outdated
Show resolved
Hide resolved
mantis-server/mantis-server-worker/src/test/java/io/mantisrx/server/worker/TestBlobStore.java
Outdated
Show resolved
Hide resolved
| } else { | ||
| String destDirStr = getDestDir(zipFile); | ||
| File destDir = new File(destDirStr); | ||
| if (!destDir.exists()) { |
There was a problem hiding this comment.
I feel like if the file path exists, we should delete it entirely and unzip zipFile there ofc logging a warning along with it.
Also, delete or close the localFile in this scenario.
There was a problem hiding this comment.
I'm now throwing an exception in case of the destDir not being empty, which is the expectation for this method.
|
|
||
| @Override | ||
| public void close() throws IOException { | ||
| blobStore.close(); |
There was a problem hiding this comment.
Should we close it or let our caller close blobStore?
There was a problem hiding this comment.
In this case, the blob store object represents the delegate that we do not want to expose to the caller to. So, we need to perform this action.
| blobStore.close(); | ||
| } | ||
|
|
||
| private String getDestDir(ZipFile zipFile) { |
There was a problem hiding this comment.
this should be called getUnzippedFile or something. Initially I got confused thinking this points to the destination directory containing the zipfile and we'll need to append something to determine the name of the unzipped file.
The name (with -unzipped suffix) would look weird it zipFile has extensions in the name but I guess that's okay for the time being.
|
Please feel free to add any comments. I'll address them in later diffs. Closing for now. |
| for (Closeable closeable: closeables) { | ||
| try { | ||
| closeable.close(); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
Just wondering if we encouter runtime exception what should be the best behavior (e.g. closeable1 failed with runtime exception, shall closeable2 still get closed)?
Two features are added through this change.
1). Firstly, I'm introducing a new blob store abstraction and a few implementations that can help download the blob from various stores such as s3, artifactory, etc.
2). A new class loader implementation has also been added to use the blob store to download the requested artifacts from s3 or artifactory or elsewhere and create a class loader object that can run the user's application code.
Context
In the new Titus world, one can use the cluster to run any user-submitted mantis job. Thus, we need capabilities to both download artifacts on demand and load them into the JVM to run the user's mantis job. This diff adds some of the abilities required for the above functionality.
Checklist
./gradlew buildcompiles code correctly./gradlew testpasses all testsCONTRIBUTING.md