[FLINK-7989][yarn] do not deploy the dist.jar file twice #4951
Conversation
| @@ -186,6 +186,8 @@ public void addShipFiles(List<File> shipFiles) { | |||
| for (File shipFile: shipFiles) { | |||
| // remove uberjar from ship list (by default everything in the lib/ folder is added to | |||
| // the list of files to ship, but we handle the uberjar separately. | |||
| // NOTE: this does not filter the uberjar if added recursively, e.g. by having the "lib" | |||
| // folder in the shipFiles | |||
| if (!(shipFile.getName().startsWith("flink-dist") && shipFile.getName().endsWith("jar"))) { | |||
There was a problem hiding this comment.
this filtering seems redundant now, the non-directory branch in uploadAndRegisterFiles should catch it.
However, I'm not sure if we should do any filtering in uploadAndRegisterFiles. We may want to pull the directory expansion into this method instead, and keep uploadAndRegisterFiles as a dumb upload method.
There was a problem hiding this comment.
oh - you're right.
Regarding the change of uploadAndRegisterFiles: I wouldn't upload the shipfiles in addShipFiles yet, so if we changed that, we'd have to traverse through the directories twice as we definitely need to do that for the uploads and if we wanted to filter recursively in addShipFiles, too, then there as well.
|
Looks good, I think we can merge it once #4939 has been merged. |
|
(rebased onto the newest #4939 which should fix the test failures shown by Travis on the previous build here) |
We always add the dist.jar ourselves, but it could also be inside a shipped folder such as the "lib/" folder and was then distributed multiple times.
- remove unnecessary code path - add documentation clearifying the methods' intentions
|
#4939 has been merged now - I rebased this PR now so we can go ahead |
|
FYI: travis errors are unrelated (ERROR 404 during download of http://mirror.netcologne.de/apache.org/kafka/0.10.2.0/kafka_2.11-0.10.2.0.tgz) |
We always add the dist.jar ourselves, but it could also be inside a shipped folder such as the "lib/" folder and was then distributed multiple times. This closes apache#4951.
We always add the dist.jar ourselves, but it could also be inside a shipped folder such as the "lib/" folder and was then distributed multiple times. This closes apache#4951.
We always add the dist.jar ourselves, but it could also be inside a shipped folder such as the "lib/" folder and was then distributed multiple times. This closes #4951.
We always add the dist.jar ourselves, but it could also be inside a shipped folder such as the "lib/" folder and was then distributed multiple times. This closes apache#4951.
What is the purpose of the change
We always add the dist.jar ourselves, but it could also be inside a shipped folder such as the "lib/" folder and was then distributed multiple times.
This PR is based on #4939 which changed the artefact uploads a bit.
Brief change log
flink-dist*.jarfrom any (recursive) upload of a shipped directory, e.g.libVerifying this change
This change is already covered by existing tests, such as any test deploying a program on YARN (if it didn't include the
flink-dist*.jarfile, it would fail). Additionally, a manual deployment showed thatflink-dist*.jaris only uploaded once.Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation