-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[MXNET-1188] Maven Publishing Test Environments #13707
Conversation
@mxnet-label-bot add [CI, Java, Maven, Scala, pr-awaiting-review] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved d4ac689 only (as requested in the description)
@frankfliu - You may be interested in this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, please address the comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. Please also apply changes to the scripts in dev
folder which reflect Frank's build changes
# specific language governing permissions and limitations | ||
# under the License. | ||
# | ||
# Dockerfile to build and run MXNet on Ubuntu 16.04 for CPU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
14.04, same as the one below
@aaronmarkham I tried a couple of time to restart the tests however it seemed not to be flaky. Could you please take a look in here? |
@@ -92,22 +92,24 @@ def get_dockerfiles_path(): | |||
|
|||
def get_platforms(path: str = get_dockerfiles_path()) -> List[str]: | |||
"""Get a list of architectures given our dockerfiles""" | |||
dockerfiles = glob.glob(os.path.join(path, "Dockerfile.build.*")) | |||
dockerfiles = glob.glob(os.path.join(path, "Dockerfile.*")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the files added by you have Dockerfile.publish as the naming convention.
Should we add ".publish" as well to the wildcard file search ? Since for the purpose of this method is to get platforms for publishing artifacts.
WDYT ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is part of the main CI code and tries to find all Dockerfiles (for caching purposes). So, it should cover both the .build and .publish dockerfiles.
I've seen this "bad address" error come up a few times recently. Restarting the CI pipeline for the failed test seems to work. I restarted it. |
set -ex | ||
apt-get update || true | ||
apt-get install -y \ | ||
build-essential \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all of this when it happens with ubuntu_core.sh? What's the difference supposed to be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is the mxnet dependencies (opencv, cuda, blas, etc.). Core should include them because it uses them to build and test mxnet throughout the CI. base should not include them because it is only for testing whether the static linking works in the publishing pipeline and the tests might erroneously pass if the dependencies are installed on the system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
base should not include them because it is only for testing whether the static linking works in the publishing pipeline and the tests might erroneously pass if the dependencies are installed on the system.
@zachgk In that case we better test on a vanilla system? Why install all the development dependencies listed in ubuntu_base.sh
if only testing the binaries? Could you help to clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is less to test binaries than to test Scala. So, we still need a few dependencies (jdk, maven, git, wget, etc). It's mostly the dependencies that are statically linked that need to be excluded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* upstream/master: (109 commits) Code modification for testcases of various network models in directory example (apache#12498) [CI] Prevent timeouts when rebuilding containers with docker. (apache#13818) fix Makefile for rpkg (apache#13590) change to compile time (apache#13835) Disabled flaky test (apache#13758) Improve license_header tool by only traversing files under revision c… (apache#13803) Removes unneeded nvidia driver ppa installation (apache#13814) Add Local test stage and option to jump directly to menu item from commandline (apache#13809) Remove MXNET_STORAGE_FALLBACK_LOG_VERBOSE from test_autograd.py (apache#13830) Fix scala doc build break for v1.3.1 (apache#13820) [MXNET-1263] Unit Tests for Java Predictor and Object Detector APIs (apache#13794) [MXNET-1260] Float64 DType computation support in Scala/Java (apache#13678) onnx export ops (apache#13821) [MXNET-880] ONNX export: Random uniform, Random normal, MaxRoiPool (apache#13676) fix minor indentation (apache#13827) Fixing a symlink issue with R install (apache#13708) remove useless code (apache#13777) ONNX ops: norm exported and lpnormalization imported (apache#13806) Add new Maven build for Scala package (apache#13819) Dockerfiles for Publish Testing (apache#13707) ...
Description
This PR is the second part of the maven Jenkins publishing that adds testing of the produced packages on 16.04, 18.04, and centOS. It also creates dedicated publishing dockerfiles (on 14.04) and testing dockerfiles that are built without any dependencies to ensure that the static linking worked properly.
Because the packages built on this do not yet use static linking, it instead tests against the snapshot repository.
Note that this PR depends on (and is rebased on top of) #13450. When reviewing the files, please look at only the changes in d4ac689.
Please review so the pipeline can be put into production:
@nswamy @lanking520 @andrewfayres @piyushghai @marcoabreu @yzhliu
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.