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 •
edited
Loading

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

@lebeg lebeg requested review from iblislin, nswamy and szha as code owners March 22, 2019 16:24
lanking520
lanking520 reviewed Mar 22, 2019
View reviewed changes
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 •
edited
Loading

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
Merged
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
perdasilva
perdasilva approved these changes Mar 25, 2019
View reviewed changes
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 lebeg mentioned this pull request Mar 25, 2019
Closed
@lebeg
Copy link
Contributor Author

lebeg commented Mar 25, 2019 •
edited
Loading

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
gavinmbell
gavinmbell approved these changes Mar 25, 2019
View reviewed changes
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

@lebeg
Fixes for data links
d5cfe8d 
* 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 •
edited
Loading

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://data.dmlc.ml/models/imagenet/nin/ in tools/coreml/README.md.

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

lanking520
lanking520 reviewed Mar 25, 2019
View reviewed changes
tools/dependencies/make_shared_dependencies.sh
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! 👍

lanking520
lanking520 approved these changes Mar 25, 2019
View reviewed changes
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

lanking520
lanking520 suggested changes Mar 25, 2019
View reviewed changes
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

cpp-package/example/get_data.sh
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
Merge pull request #17 from apache/master
2501376 
Fixes for CI downloads (apache#14504)
This was referenced Mar 26, 2019
Merged
Merged
@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
@lebeg @vdantu
Fixes for CI downloads (apache#14504)
1de184b 
* 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
@lebeg @ZhennanQin
Fixes for CI downloads (apache#14504)
5af377c 
* 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
@lebeg @nswamy
Fixes for CI downloads (#14504)
ce5bc19 
* 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 •
edited
Loading

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
@lebeg
Fixes for CI downloads (apache#14504)
27921d7 
* 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.
Reviewers

@lanking520 lanking520 lanking520 approved these changes

@gavinmbell gavinmbell gavinmbell approved these changes

@perdasilva perdasilva perdasilva approved these changes

@iblislin iblislin iblislin approved these changes

@nswamy nswamy Awaiting requested review from nswamy nswamy is a code owner

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
CI pr-awaiting-response PR is reviewed and waiting for contributor to respond
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@lebeg @lanking520 @pinaraws @gavinmbell @kwanking @iblislin @perdasilva @jnorwood @marcoabreu