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

enable loading config.yml files from URL #3977

Merged
merged 1 commit into from
Jun 2, 2020
Merged
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
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@
(?x)^(
aiida/cmdline/commands/.*|
aiida/cmdline/params/.*|
aiida/cmdline/params/types/.*|
ltalirz marked this conversation as resolved.
Show resolved Hide resolved
utils/validate_consistency.py|
)$
pass_filenames: false
Expand Down
4 changes: 2 additions & 2 deletions aiida/cmdline/commands/cmd_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from aiida.cmdline.commands.cmd_verdi import verdi
from aiida.cmdline.params import options
from aiida.cmdline.params.types import GroupParamType, ImportPath
from aiida.cmdline.params.types import GroupParamType, PathOrUrl
from aiida.cmdline.utils import decorators, echo

EXTRAS_MODE_EXISTING = ['keep_existing', 'update_existing', 'mirror', 'none', 'ask']
Expand Down Expand Up @@ -186,7 +186,7 @@ def _migrate_archive(ctx, temp_folder, file_to_import, archive, non_interactive,


@verdi.command('import')
@click.argument('archives', nargs=-1, type=ImportPath(exists=True, readable=True))
@click.argument('archives', nargs=-1, type=PathOrUrl(exists=True, readable=True))
@click.option(
'-w',
'--webpages',
Expand Down
4 changes: 2 additions & 2 deletions aiida/cmdline/params/options/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,8 @@ def decorator(command):

CONFIG_FILE = ConfigFileOption(
'--config',
type=click.Path(exists=True, dir_okay=False),
help='Load option values from configuration file in yaml format.'
type=types.FileOrUrl(),
help='Load option values from configuration file in yaml format (local path or URL).'
)

IDENTIFIER = OverridableOption(
Expand Down
7 changes: 3 additions & 4 deletions aiida/cmdline/params/options/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@
from .overridable import OverridableOption


def yaml_config_file_provider(file_path, cmd_name): # pylint: disable=unused-argument
"""Read yaml config file."""
with open(file_path, 'r') as handle:
return yaml.safe_load(handle)
def yaml_config_file_provider(handle, cmd_name): # pylint: disable=unused-argument
"""Read yaml config file from file handle."""
return yaml.safe_load(handle)


class ConfigFileOption(OverridableOption):
Expand Down
5 changes: 3 additions & 2 deletions aiida/cmdline/params/types/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from .node import NodeParamType
from .process import ProcessParamType
from .strings import (NonEmptyStringParamType, EmailType, HostnameType, EntryPointType, LabelStringType)
from .path import AbsolutePathParamType, ImportPath
from .path import AbsolutePathParamType, PathOrUrl, FileOrUrl
from .plugin import PluginParamType
from .profile import ProfileParamType
from .user import UserParamType
Expand All @@ -32,5 +32,6 @@
'LazyChoice', 'IdentifierParamType', 'CalculationParamType', 'CodeParamType', 'ComputerParamType',
'ConfigOptionParamType', 'DataParamType', 'GroupParamType', 'NodeParamType', 'MpirunCommandParamType',
'MultipleValueParamType', 'NonEmptyStringParamType', 'PluginParamType', 'AbsolutePathParamType', 'ShebangParamType',
'UserParamType', 'TestModuleParamType', 'ProfileParamType', 'WorkflowParamType', 'ProcessParamType', 'ImportPath'
'UserParamType', 'TestModuleParamType', 'ProfileParamType', 'WorkflowParamType', 'ProcessParamType', 'PathOrUrl',
'FileOrUrl'
)
95 changes: 64 additions & 31 deletions aiida/cmdline/params/types/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,25 @@
# See https://stackoverflow.com/a/41217363/1069467
import urllib.request
import urllib.error

from socket import timeout
import click

URL_TIMEOUT_SECONDS = 10


def _check_timeout_seconds(timeout_seconds):
"""Raise if timeout is not within range [0;60]"""
try:
timeout_seconds = int(timeout_seconds)
except ValueError:
raise TypeError('timeout_seconds should be an integer but got: {}'.format(type(timeout_seconds)))

if timeout_seconds < 0 or timeout_seconds > 60:
raise ValueError('timeout_seconds needs to be in the range [0;60].')

return timeout_seconds


class AbsolutePathParamType(click.Path):
"""
The ParamType for identifying absolute Paths (derived from click.Path).
Expand Down Expand Up @@ -52,22 +65,23 @@ def __repr__(self):
return 'ABSOLUTEPATHEMPTY'


class ImportPath(click.Path):
"""AiiDA extension of Click's Path-type to include URLs
An ImportPath can either be a `click.Path`-type or a URL.
class PathOrUrl(click.Path):
"""Extension of click's Path-type to include URLs.

A PathOrUrl can either be a `click.Path`-type or a URL.

:param timeout_seconds: Timeout time in seconds that a URL response is expected.
:value timeout_seconds: Must be an int in the range [0;60], extrema included.
If an int outside the range [0;60] is given, the value will be set to the respective extremum value.
If any other type than int is given a TypeError will be raised.
:param int timeout_seconds: Maximum timeout accepted for URL response.
Must be an integer in the range [0;60].
"""

# pylint: disable=protected-access

name = 'PathOrUrl'

def __init__(self, timeout_seconds=URL_TIMEOUT_SECONDS, **kwargs):
super().__init__(**kwargs)

self.timeout_seconds = timeout_seconds
self.timeout_seconds = _check_timeout_seconds(timeout_seconds)

def convert(self, value, param, ctx):
"""Overwrite `convert`
Expand All @@ -80,38 +94,57 @@ def convert(self, value, param, ctx):
# Check if URL
return self.checks_url(value, param, ctx)

def checks_url(self, value, param, ctx):
"""Do checks for possible URL path"""
from socket import timeout

url = value

def checks_url(self, url, param, ctx):
"""Check whether URL is reachable within timeout."""
try:
urllib.request.urlopen(url, data=None, timeout=self.timeout_seconds)
urllib.request.urlopen(url, timeout=self.timeout_seconds)
except (urllib.error.URLError, urllib.error.HTTPError, timeout):
self.fail(
'{0} "{1}" could not be reached within {2} s.\n'
'It may be neither a valid {3} nor a valid URL.'.format(
'Is it a valid {3} or URL?'.format(
self.path_type, click._compat.filename_to_ui(url), self.timeout_seconds, self.name
), param, ctx
)

return url

@property
def timeout_seconds(self):
return self._timeout_seconds

# pylint: disable=attribute-defined-outside-init
@timeout_seconds.setter
def timeout_seconds(self, value):
try:
self._timeout_seconds = int(value)
except ValueError:
raise TypeError('timeout_seconds should be an integer but got: {}'.format(type(value)))
class FileOrUrl(click.File):
"""Extension of click's File-type to include URLs.

Returns handle either to local file or to remote file fetched from URL.

:param int timeout_seconds: Maximum timeout accepted for URL response.
Must be an integer in the range [0;60].
"""

name = 'FileOrUrl'

# pylint: disable=protected-access

def __init__(self, timeout_seconds=URL_TIMEOUT_SECONDS, **kwargs):
super().__init__(**kwargs)

if self._timeout_seconds < 0:
self._timeout_seconds = 0
self.timeout_seconds = _check_timeout_seconds(timeout_seconds)

if self._timeout_seconds > 60:
self._timeout_seconds = 60
def convert(self, value, param, ctx):
"""Return file handle.
"""
try:
# Check if `click.File`-type
return super().convert(value, param, ctx)
except click.exceptions.BadParameter:
# Check if URL
handle = self.get_url(value, param, ctx)
return handle

def get_url(self, url, param, ctx):
"""Retrieve file from URL."""
try:
return urllib.request.urlopen(url, timeout=self.timeout_seconds)
except (urllib.error.URLError, urllib.error.HTTPError, timeout):
self.fail(
'"{0}" could not be reached within {1} s.\n'
'Is it a valid {2} or URL?.'.format(click._compat.filename_to_ui(url), self.timeout_seconds, self.name),
param, ctx
)
2 changes: 1 addition & 1 deletion aiida/common/extendeddicts.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
__all__ = ('AttributeDict', 'FixedFieldsAttributeDict', 'DefaultFieldsAttributeDict')


class AttributeDict(dict):
class AttributeDict(dict): # pylint: disable=too-many-instance-attributes
"""
This class internally stores values in a dictionary, but exposes
the keys also as attributes, i.e. asking for attrdict.key
Expand Down
2 changes: 1 addition & 1 deletion docs/requirements_for_rtd.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ alembic~=1.2
ase~=3.18
circus~=0.16.1
click-completion~=0.5.1
click-config-file~=0.5.0
click-config-file~=0.6.0
click-spinner~=0.1.8
click~=7.0
coverage<5.0
Expand Down
1 change: 1 addition & 0 deletions docs/source/nitpick-exceptions
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ py:class click.types.Choice
py:class click.types.IntParamType
py:class click.types.StringParamType
py:class click.types.Path
py:class click.types.File
py:meth click.Option.get_default

py:class concurrent.futures._base.TimeoutError
Expand Down
8 changes: 4 additions & 4 deletions docs/source/reference/command_line.rst
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,8 @@ Below is a list with all available subcommands.
superuser.

--repository DIRECTORY Absolute path to the file repository.
--config FILE Load option values from configuration file
in yaml format.
--config FILEORURL Load option values from configuration file
in yaml format (local path or URL).

--help Show this message and exit.

Expand Down Expand Up @@ -622,8 +622,8 @@ Below is a list with all available subcommands.

--db-password TEXT Password of the database user. [required]
--repository DIRECTORY Absolute path to the file repository.
--config FILE Load option values from configuration file
in yaml format.
--config FILEORURL Load option values from configuration file
in yaml format (local path or URL).

--help Show this message and exit.

Expand Down
2 changes: 1 addition & 1 deletion environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ dependencies:
- alembic~=1.2
- circus~=0.16.1
- click-completion~=0.5.1
- click-config-file~=0.5.0
- click-config-file~=0.6.0
- click-spinner~=0.1.8
- click~=7.0
- django~=2.2
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.5.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ chardet==3.0.4
circus==0.16.1
Click==7.0
click-completion==0.5.2
click-config-file==0.5.0
click-config-file==0.6.0
click-spinner==0.1.8
configobj==5.0.6
coverage==4.5.4
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.6.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ chardet==3.0.4
circus==0.16.1
Click==7.0
click-completion==0.5.2
click-config-file==0.5.0
click-config-file==0.6.0
click-spinner==0.1.8
configobj==5.0.6
coverage==4.5.4
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.7.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ chardet==3.0.4
circus==0.16.1
Click==7.0
click-completion==0.5.2
click-config-file==0.5.0
click-config-file==0.6.0
click-spinner==0.1.8
configobj==5.0.6
coverage==4.5.4
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements-py-3.8.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ chardet==3.0.4
circus==0.16.1
Click==7.0
click-completion==0.5.2
click-config-file==0.5.0
click-config-file==0.6.0
click-spinner==0.1.8
configobj==5.0.6
coverage==4.5.4
Expand Down
2 changes: 1 addition & 1 deletion setup.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"alembic~=1.2",
"circus~=0.16.1",
"click-completion~=0.5.1",
"click-config-file~=0.5.0",
"click-config-file~=0.6.0",
"click-spinner~=0.1.8",
"click~=7.0",
"django~=2.2",
Expand Down
35 changes: 25 additions & 10 deletions tests/cmdline/commands/test_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import subprocess as sp
from textwrap import dedent

from unittest import mock
from click.testing import CliRunner
import pytest

Expand Down Expand Up @@ -69,32 +70,46 @@ def test_noninteractive_upload(self):
self.assertIsInstance(orm.Code.get_from_string('{}'.format(label)), orm.Code)

def test_from_config(self):
"""Test setting up a code from a config file"""
import tempfile
"""Test setting up a code from a config file.

label = 'noninteractive_config'
Try loading from local file and from URL.
"""
import tempfile

with tempfile.NamedTemporaryFile('w') as handle:
handle.write(
dedent(
"""
config_file_template = dedent(
"""
---
label: {label}
input_plugin: arithmetic.add
computer: {computer}
remote_abs_path: /remote/abs/path
"""
).format(label=label, computer=self.computer.name)
)
)

# local file
label = 'noninteractive_config'
with tempfile.NamedTemporaryFile('w') as handle:
handle.write(config_file_template.format(label=label, computer=self.computer.name))
handle.flush()
result = self.cli_runner.invoke(
setup_code,
['--non-interactive', '--config', os.path.realpath(handle.name)]
)

self.assertClickResultNoException(result)
self.assertIsInstance(orm.Code.get_from_string('{}'.format(label)), orm.Code)

# url
label = 'noninteractive_config_url'
fake_url = 'https://my.url.com'
with mock.patch(
'urllib.request.urlopen',
return_value=config_file_template.format(label=label, computer=self.computer.name)
):
result = self.cli_runner.invoke(setup_code, ['--non-interactive', '--config', fake_url])

self.assertClickResultNoException(result)
self.assertIsInstance(orm.Code.get_from_string('{}'.format(label)), orm.Code)


class TestVerdiCodeCommands(AiidaTestCase):
"""Testing verdi code commands.
Expand Down
6 changes: 3 additions & 3 deletions tests/cmdline/commands/test_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,11 @@ def test_import_url_and_local_archives(self):

def test_import_url_timeout(self):
"""Test a timeout to valid URL is correctly errored"""
from aiida.cmdline.params.types import ImportPath
from aiida.cmdline.params.types import PathOrUrl

timeout_url = 'http://www.google.com:81'

test_timeout_path = ImportPath(exists=True, readable=True, timeout_seconds=0)
test_timeout_path = PathOrUrl(exists=True, readable=True, timeout_seconds=0)
with self.assertRaises(BadParameter) as cmd_exc:
test_timeout_path(timeout_url)

Expand All @@ -229,7 +229,7 @@ def test_raise_malformed_url(self):
self.assertIsNotNone(result.exception, result.output)
self.assertNotEqual(result.exit_code, 0, result.output)

error_message = 'It may be neither a valid path nor a valid URL.'
error_message = 'Is it a valid path or URL?'
self.assertIn(error_message, result.output, result.exception)

def test_non_interactive_and_migration(self):
Expand Down
Loading