Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Fixes for CI downloads #14504

Merged
merged 2 commits into from
Mar 26, 2019
Merged

Fixes for CI downloads #14504

merged 2 commits into from
Mar 26, 2019

Conversation

lebeg
Copy link
Contributor

@lebeg lebeg commented Mar 22, 2019

Description

Fixed for CI verification builds.

Changes

  • Checked that silent curl downloads succeeded
  • Added curl download retries
  • Replaced model download links from dmlc to mxnet

Copy link
Member

@lanking520 lanking520 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your effort! Do we have default CURL option specified in somewhere?

@lebeg
Copy link
Contributor Author

lebeg commented Mar 22, 2019

Yes, you can find it here. There seem to be other issues still, will need to take a look into those as well.

@lanking520
Copy link
Member

@lebeg The static build failure was caused by no default CURL option specified. Without -s -L, it won't work properly.

@lebeg
Copy link
Contributor Author

lebeg commented Mar 22, 2019

Sorry, I didn’t understand - you are saying that it wouldn’t work without -s ?
The -L option is specified in the template by —local.

@lanking520
Copy link
Member

Sorry, I didn’t understand - you are saying that it wouldn’t work without -s ?
The -L option is specified in the template by —local.

Might be -s. Please put the environment variable into make_shared_dependeices.sh

@ZhennanQin ZhennanQin mentioned this pull request Mar 23, 2019
7 tasks
@pinaraws
Copy link

@mxnet-label-bot add[pr-awaiting-response]

@marcoabreu marcoabreu added the pr-awaiting-response PR is reviewed and waiting for contributor to respond label Mar 25, 2019
@pinaraws
Copy link

@mxnet-label-bot add[CI]

@marcoabreu marcoabreu added the CI label Mar 25, 2019
Copy link
Contributor

@perdasilva perdasilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lebeg
Copy link
Contributor Author

lebeg commented Mar 25, 2019

Might be -s. Please put the environment variable into make_shared_dependeices.sh

-s is for 'silent' and is the exact reason why CI is not propagating the download error correctly. You can see options which are set by a template for all downloads in make_shared_dependeices.sh

@lebeg lebeg force-pushed the fix_ci branch 2 times, most recently from a31bfbd to 7d47521 Compare March 25, 2019 09:55
Copy link

@gavinmbell gavinmbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

* Changed data.dmlc.ml -> data.mxnet.io
* Added retries for downloads
* Removed silent mode for explicit failure reporting
@lebeg
Copy link
Contributor Author

lebeg commented Mar 25, 2019

The -L option is specified in the template by —local.

@lanking520 Sorry, I think I caused some confusion, I meant the --location option. I brought the --silent option back, but made a check for the downloaded file.

@lebeg
Copy link
Contributor Author

lebeg commented Mar 25, 2019

These still have references to that data.dmlc.ml broken link

tools/coreml/README.md

example/rcnn/README.md

docs/model_zoo/index.md

tools/coreml/test/test_mxnet_models.py

R-package/tests/testthat/get_data.R

julia/models/Inception/get.sh

cpp-package/example/feature_extract/run.sh

I could just change data.dmlc.ml to data.mxnet.io in the README files, but I have checked that some folders mentioned don't exist. For example http:https://data.dmlc.ml/models/imagenet/nin/ in tools/coreml/README.md.

I'm not sure I want to make those changes in this PR.

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"

download () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome config! 👍

Copy link
Member

@lanking520 lanking520 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this PR, let's see if we can pass CI this time

Copy link
Member

@lanking520 lanking520 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Message came out from here is not right

curl --connect-timeout 10 --max-time 300 --retry-delay 10 --retry 3 --retry-delay 0 --location --silent https://github.com/glennrp/libpng/archive/v1.6.34.zip -o -o

mkdir -p data/mnist_data
cd data/mnist_data

download () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed not properly configured the file name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error was from incorrect -o supplied to the download function.

@gavinmbell
Copy link

There are two download() functions here. Can we put that download function in a library type of script and then use it accordingly - to adhere to a DRY (Don't Repeat Yourself) sensibility?

The diff seems to be with unzipping activities. I realize one is in 'examples' (cpp-package/example/get_data.sh) and the other under tools (tools/dependencies/make_shared_dependencies.sh) - perhaps these are disparate enough things to warrant a copy and paste - I don't know. Just flagging this.

@lanking520 lanking520 merged commit a9458ca into apache:master Mar 26, 2019
lihaofd pushed a commit to lihaofd/incubator-mxnet that referenced this pull request Mar 26, 2019
@lebeg
Copy link
Contributor Author

lebeg commented Mar 26, 2019

Created ports for release branches:

  • #14525 Fixes for CI downloads (v1.3.x)
  • #14526 Fixes for CI downloads (v1.4.x)

vdantu pushed a commit to vdantu/incubator-mxnet that referenced this pull request Mar 31, 2019
* Fixes for data links

* Changed data.dmlc.ml -> data.mxnet.io
* Added retries for downloads
* Removed silent mode for explicit failure reporting

* Reworked download scripts
ZhennanQin pushed a commit to ZhennanQin/incubator-mxnet that referenced this pull request Apr 3, 2019
* Fixes for data links

* Changed data.dmlc.ml -> data.mxnet.io
* Added retries for downloads
* Removed silent mode for explicit failure reporting

* Reworked download scripts
nswamy pushed a commit that referenced this pull request Apr 5, 2019
* Fixes for data links

* Changed data.dmlc.ml -> data.mxnet.io
* Added retries for downloads
* Removed silent mode for explicit failure reporting

* Reworked download scripts
@kwanking
Copy link

kwanking commented Apr 24, 2019

when run ci/build.py, there is a error, how let timeout be long? thanks!

build.py: 2019-04-24 13:38:36,494Z WARNING Exception: Command '['docker', 'pull', 'mxnetci/build.android_armv7']' timed out after 900 seconds, Retrying in 50 seconds..

haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* Fixes for data links

* Changed data.dmlc.ml -> data.mxnet.io
* Added retries for downloads
* Removed silent mode for explicit failure reporting

* Reworked download scripts
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI pr-awaiting-response PR is reviewed and waiting for contributor to respond
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants