Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --download-timeout parser options #987

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/rosdep2/gbpdistro_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@
FUERTE_GBPDISTRO_URL = 'https://raw.githubusercontent.com/ros/rosdistro/' \
'master/releases/fuerte.yaml'

# seconds to wait before aborting download of gbpdistro data
DOWNLOAD_TIMEOUT = 15.0


def get_owner_name(url):
"""
Expand Down Expand Up @@ -181,7 +178,7 @@ def get_gbprepo_as_rosdep_data(gbpdistro):
return rosdep_data


def download_gbpdistro_as_rosdep_data(gbpdistro_url, targets_url=None):
def download_gbpdistro_as_rosdep_data(gbpdistro_url, timeout, targets_url=None):
"""
Download gbpdistro file from web and convert format to rosdep distro data.

Expand All @@ -197,9 +194,9 @@ def download_gbpdistro_as_rosdep_data(gbpdistro_url, targets_url=None):
# we can convert a gbpdistro file into rosdep data by following a
# couple rules
# will output a warning
targets_data = download_targets_data(targets_url=targets_url)
targets_data = download_targets_data(targets_url=targets_url, timeout=timeout)
try:
f = urlopen_gzip(gbpdistro_url, timeout=DOWNLOAD_TIMEOUT)
f = urlopen_gzip(gbpdistro_url, timeout=timeout)
text = f.read()
f.close()
gbpdistro_data = yaml.safe_load(text)
Expand Down
8 changes: 7 additions & 1 deletion src/rosdep2/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,11 @@ def _rosdep_main(args):
default=[], action='append',
help='Dependency types to install, can be given multiple times. '
'Choose from {}. Default: all except doc.'.format(VALID_DEPENDENCY_TYPES))
parser.add_option("--download-timeout", dest="download_timeout",
type=float, default=15.0,
help="Affects the 'update' verb. "
"Timeout in seconds for pull source list data. "
"Default to 15.0 seconds.")

options, args = parser.parse_args(args)
if options.print_version or options.print_all_versions:
Expand Down Expand Up @@ -598,7 +603,7 @@ def configure_installer_context(installer_context, options):

def command_init(options):
try:
data = download_default_sources_list()
data = download_default_sources_list(options.download_timeout)
except URLError as e:
print('ERROR: cannot download default sources list from:\n%s\nWebsite may be down.' % (DEFAULT_SOURCES_LIST_URL), file=sys.stderr)
print(e, file=sys.stderr)
Expand Down Expand Up @@ -671,6 +676,7 @@ def update_error_handler(data_source, exc):
error_handler=update_error_handler,
skip_eol_distros=not options.include_eol_distros,
ros_distro=options.ros_distro,
download_timeout=options.download_timeout,
quiet=options.quiet)
if not options.quiet:
print('updated cache in %s' % (sources_cache_dir))
Expand Down
7 changes: 2 additions & 5 deletions src/rosdep2/rep3.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,8 @@
# location of targets file for processing gbpdistro files
REP3_TARGETS_URL = 'https://raw.githubusercontent.com/ros/rosdistro/master/releases/targets.yaml'

# seconds to wait before aborting download of gbpdistro data
DOWNLOAD_TIMEOUT = 15.0


def download_targets_data(targets_url=None):
def download_targets_data(targets_url=None, timeout=15.0):
"""
Download REP 3 targets file and unmarshal from YAML.
DEPRECATED: this function is deprecated. List of targets should be obtained
Expand All @@ -56,7 +53,7 @@ def download_targets_data(targets_url=None):
if targets_url is None:
targets_url = REP3_TARGETS_URL
try:
f = urlopen_gzip(targets_url, timeout=DOWNLOAD_TIMEOUT)
f = urlopen_gzip(targets_url, timeout=timeout)
text = f.read()
f.close()
targets_data = yaml.safe_load(text)
Expand Down
18 changes: 9 additions & 9 deletions src/rosdep2/sources_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@
# rosdep
DEFAULT_SOURCES_LIST_URL = 'https://raw.githubusercontent.com/ros/rosdistro/master/rosdep/sources.list.d/20-default.list'

# seconds to wait before aborting download of rosdep data
DOWNLOAD_TIMEOUT = 15.0

SOURCES_LIST_DIR = 'sources.list.d'
SOURCES_CACHE_DIR = 'sources.cache'

Expand Down Expand Up @@ -292,13 +289,13 @@ def create_default(os_override=None):
return DataSourceMatcher(tags)


def download_rosdep_data(url):
def download_rosdep_data(url, timeout):
"""
:raises: :exc:`DownloadFailure` If data cannot be
retrieved (e.g. 404, bad YAML format, server down).
"""
try:
f = urlopen_gzip(url, timeout=DOWNLOAD_TIMEOUT)
f = urlopen_gzip(url, timeout=timeout)
text = f.read()
f.close()
data = yaml.safe_load(text)
Expand All @@ -311,10 +308,11 @@ def download_rosdep_data(url):
raise DownloadFailure(str(e))


def download_default_sources_list(url=DEFAULT_SOURCES_LIST_URL):
def download_default_sources_list(timeout, url=DEFAULT_SOURCES_LIST_URL):
"""
Download (and validate) contents of default sources list.

:param timeout: download timeout in seconds
:param url: override URL of default sources list file
:return: raw sources list data, ``str``
:raises: :exc:`DownloadFailure` If data cannot be
Expand All @@ -323,7 +321,7 @@ def download_default_sources_list(url=DEFAULT_SOURCES_LIST_URL):
retrieved (e.g. 404, server down).
"""
try:
f = urlopen_gzip(url, timeout=DOWNLOAD_TIMEOUT)
f = urlopen_gzip(url, timeout=timeout)
except (URLError, httplib.HTTPException) as e:
raise URLError(str(e) + ' (%s)' % url)
data = f.read().decode()
Expand Down Expand Up @@ -435,6 +433,7 @@ def _generate_key_from_urls(urls):
def update_sources_list(sources_list_dir=None, sources_cache_dir=None,
success_handler=None, error_handler=None,
skip_eol_distros=False, ros_distro=None,
download_timeout=15.0,
quiet=False):
"""
Re-downloaded data from remote sources and store in cache. Also
Expand All @@ -449,6 +448,7 @@ def update_sources_list(sources_list_dir=None, sources_cache_dir=None,
if a particular source fails. This hook is mainly for
printing errors to console.
:param skip_eol_distros: skip downloading sources for EOL distros
:param timeout: download timeout in seconds, default to 15.0

:returns: list of (`DataSource`, cache_file_path) pairs for cache
files that were updated, ``[str]``
Expand All @@ -464,14 +464,14 @@ def update_sources_list(sources_list_dir=None, sources_cache_dir=None,
for source in list(sources):
try:
if source.type == TYPE_YAML:
rosdep_data = download_rosdep_data(source.url)
rosdep_data = download_rosdep_data(source.url, download_timeout)
elif source.type == TYPE_GBPDISTRO: # DEPRECATED, do not use this file. See REP137
if not source.tags[0] in ['electric', 'fuerte']:
if not quiet:
print('Ignore legacy gbpdistro "%s"' % source.tags[0])
sources.remove(source)
continue # do not store this entry in the cache
rosdep_data = download_gbpdistro_as_rosdep_data(source.url)
rosdep_data = download_gbpdistro_as_rosdep_data(source.url, download_timeout)
retval.append((source, write_cache_file(sources_cache_dir, source.url, rosdep_data)))
if success_handler is not None:
success_handler(source)
Expand Down
5 changes: 3 additions & 2 deletions test/test_rosdep_gbpdistro_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def test_download_gbpdistro_as_rosdep_data():
from rosdep2.gbpdistro_support import FUERTE_GBPDISTRO_URL
from rosdep2.rep3 import REP3_TARGETS_URL
from rosdep2 import DownloadFailure
data = download_gbpdistro_as_rosdep_data(FUERTE_GBPDISTRO_URL)
data = download_gbpdistro_as_rosdep_data(FUERTE_GBPDISTRO_URL, 15.0)
# don't go beyond this, this test is just making sure the download
# plumbing is correct, not the loader.
for k in ['ros', 'catkin', 'genmsg']:
Expand All @@ -89,13 +89,14 @@ def test_download_gbpdistro_as_rosdep_data():
# override targets URL with bad URL
download_gbpdistro_as_rosdep_data(
FUERTE_GBPDISTRO_URL,
15.0,
targets_url='http://bad.ros.org/foo.yaml')
assert False, 'should have raised'
except DownloadFailure:
pass
try:
# use targets URL, which should have a bad format
download_gbpdistro_as_rosdep_data(REP3_TARGETS_URL)
download_gbpdistro_as_rosdep_data(REP3_TARGETS_URL, 15.0)
assert False, 'should have raised'
except DownloadFailure:
pass
Expand Down
1 change: 1 addition & 0 deletions test/test_rosdep_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ def read_stdout(cmd, capture_stderr=False):
'install', '-s', '-i',
'--os', 'ubuntu:lucid',
'--rosdistro', 'fuerte',
'--download-timeout', '15.0',
'--from-paths', catkin_tree
] + cmd_extras)
stdout, stderr = b
Expand Down
7 changes: 7 additions & 0 deletions test/test_rosdep_rep3.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,10 @@ def test_download_targets_data():
assert False, 'should have raised'
except DownloadFailure:
pass

# try timeout
try:
download_targets_data(targets_url=REP3_TARGETS_URL, timeout=0.0001)
assert False, 'should have raised'
except DownloadFailure:
pass
35 changes: 30 additions & 5 deletions test/test_rosdep_sources_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,16 @@ def test_url_constants():
@pytest.mark.online
def test_download_default_sources_list():
from rosdep2.sources_list import download_default_sources_list
data = download_default_sources_list()
data = download_default_sources_list(15.0)
assert 'http' in data, data # sanity check, all sources files have urls
try:
download_default_sources_list(url='http://bad.ros.org/foo.yaml')
download_default_sources_list(15.0, url='http://bad.ros.org/foo.yaml')
assert False, 'should not have succeeded/valdiated'
except URLError:
pass

try:
download_default_sources_list(0.0001, url='http://bad.ros.org/foo.yaml')
assert False, 'should not have succeeded/valdiated'
except URLError:
pass
Expand Down Expand Up @@ -248,6 +254,16 @@ def error_handler(loc, e):
'yaml %s ubuntu' % (GITHUB_URL, GITHUB_PYTHON_URL, BADHOSTNAME_URL)
assert expected == index, '\n[%s]\nvs\n[%s]' % (expected, index)

# test timeout
errors = []

def error_handler(loc, e):
errors.append((loc, e))
retval = update_sources_list(sources_list_dir=sources_list_dir,
sources_cache_dir=tempdir, error_handler=error_handler, download_timeout=0.0001)
print(retval)
print(errors)


@pytest.mark.online
def test_load_cached_sources_list():
Expand Down Expand Up @@ -306,22 +322,31 @@ def test_download_rosdep_data():
from rosdep2.sources_list import download_rosdep_data
from rosdep2 import DownloadFailure
url = GITHUB_BASE_URL
data = download_rosdep_data(url)
data = download_rosdep_data(url, 15.0)
assert 'boost' in data # sanity check

# try with a bad URL
try:
data = download_rosdep_data('http://badhost.willowgarage.com/rosdep.yaml')
data = download_rosdep_data('http://badhost.willowgarage.com/rosdep.yaml', 15.0)
assert False, 'should have raised'
except DownloadFailure as e:
pass

# test timeout
url = GITHUB_BASE_URL
try:
data = download_rosdep_data(url, 0.0001)
assert False, 'should have raised'
except DownloadFailure as e:
pass

# try to trigger both non-dict clause and YAMLError clause
for url in [
'https://code.ros.org/svn/release/trunk/distros/',
'https://code.ros.org/svn/release/trunk/distros/manifest.xml',
]:
try:
data = download_rosdep_data(url)
data = download_rosdep_data(url, 15.0)
assert False, 'should have raised'
except DownloadFailure as e:
pass
Expand Down