Skip to content

Commit

Permalink
Remove use of legacy Code
Browse files Browse the repository at this point in the history
The `Code` plugin is used a lot and so cannot just be removed. Instead,
we keep it for now and have it subclass the new `AbstractCode`. This
way, the class already has the interface that it will have once the
deprecated interface is removed. This will allow users to start updating
their use of it in a backwards-compatible manner.

The codebase is updated to replace the use of the deprecated interface
with that of the new interface. No new instances of `Code` are created,
only instances of the new `RemoteCode` and `LocalCode`.

When the time comes to drop the legacy `Code` class, all that is needed
is a data migration that will update existing instances to either an
instance of `InstalledCode` or `PortableCode` depending on the attributes
that the instance defines.

The legacy methods and properties of `Code` plugin are deprecated.
  • Loading branch information
sphuber committed May 18, 2022
1 parent c933f58 commit a2f6d79
Show file tree
Hide file tree
Showing 34 changed files with 316 additions and 297 deletions.
6 changes: 4 additions & 2 deletions .github/system_tests/pytest/test_memory_leaks.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ def test_leak_ssh_calcjob():
Note: This relies on the 'slurm-ssh' computer being set up.
"""
code = orm.Code(
input_plugin_name='core.arithmetic.add', remote_computer_exec=[orm.load_computer('slurm-ssh'), '/bin/bash']
code = orm.InstalledCode(
default_calc_job_plugin='core.arithmetic.add',
computer=orm.load_computer('slurm-ssh'),
filepath_executable='/bin/bash'
)
inputs = {'x': orm.Int(1), 'y': orm.Int(2), 'code': code}
run_finished_ok(ArithmeticAddCalculation, **inputs)
Expand Down
42 changes: 14 additions & 28 deletions aiida/cmdline/commands/cmd_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ def setup_code(ctx, non_interactive, **kwargs):
except Exception as exception: # pylint: disable=broad-except
echo.echo_critical(f'Unable to store the Code: {exception}')

code.reveal()
echo.echo_success(f'Code<{code.pk}> {code.full_label} created')


Expand All @@ -127,9 +126,11 @@ def code_test(code):
* Whether the remote executable exists.
"""
if not code.is_local():
from aiida.orm import InstalledCode

if isinstance(code, InstalledCode):
try:
code.validate_remote_exec_path()
code.validate_filepath_executable()
except exceptions.ValidationError as exception:
echo.echo_critical(f'validation failed: {exception}')

Expand Down Expand Up @@ -169,10 +170,11 @@ def code_duplicate(ctx, code, non_interactive, **kwargs):
kwargs['code_type'] = CodeBuilder.CodeType.STORE_AND_UPLOAD

if kwargs.pop('hide_original'):
code.hide()
code.is_hidden = True

# Convert entry point to its name
kwargs['input_plugin'] = kwargs['input_plugin'].name
if kwargs['input_plugin']:
kwargs['input_plugin'] = kwargs['input_plugin'].name

code_builder = ctx.code_builder
for key, value in kwargs.items():
Expand All @@ -181,7 +183,7 @@ def code_duplicate(ctx, code, non_interactive, **kwargs):

try:
new_code.store()
new_code.reveal()
new_code.is_hidden = False
except ValidationError as exception:
echo.echo_critical(f'Unable to store the Code: {exception}')

Expand All @@ -194,31 +196,15 @@ def code_duplicate(ctx, code, non_interactive, **kwargs):
def show(code):
"""Display detailed information for a code."""
from aiida.cmdline import is_verbose
from aiida.repository import FileType

table = []
table.append(['PK', code.pk])
table.append(['UUID', code.uuid])
table.append(['Label', code.label])
table.append(['Description', code.description])
table.append(['Default plugin', code.get_input_plugin_name()])

if code.is_local():
table.append(['Type', 'local'])
table.append(['Exec name', code.get_execname()])
table.append(['List of files/folders:', ''])
for obj in code.list_objects():
if obj.file_type == FileType.DIRECTORY:
table.append(['directory', obj.name])
else:
table.append(['file', obj.name])
else:
table.append(['Type', 'remote'])
table.append(['Remote machine', code.get_remote_computer().label])
table.append(['Remote absolute path', code.get_remote_exec_path()])

table.append(['Prepend text', code.get_prepend_text()])
table.append(['Append text', code.get_append_text()])
table.append(['Default plugin', code.default_calc_job_plugin])
table.append(['Prepend text', code.prepend_text])
table.append(['Append text', code.append_text])

if is_verbose():
table.append(['Calculations', len(code.base.links.get_outgoing().all())])
Expand Down Expand Up @@ -258,7 +244,7 @@ def _dry_run_callback(pks):
def hide(codes):
"""Hide one or more codes from `verdi code list`."""
for code in codes:
code.hide()
code.is_hidden = True
echo.echo_success(f'Code<{code.pk}> {code.full_label} hidden')


Expand All @@ -268,7 +254,7 @@ def hide(codes):
def reveal(codes):
"""Reveal one or more hidden codes in `verdi code list`."""
for code in codes:
code.reveal()
code.is_hidden = False
echo.echo_success(f'Code<{code.pk}> {code.full_label} revealed')


Expand All @@ -281,7 +267,7 @@ def relabel(code, label):
old_label = code.full_label

try:
code.relabel(label)
code.label = label
except (TypeError, ValueError) as exception:
echo.echo_critical(f'invalid code label: {exception}')
else:
Expand Down
2 changes: 1 addition & 1 deletion aiida/cmdline/params/types/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def convert(self, value, param, ctx):
code = super().convert(value, param, ctx)

if code and self._entry_point is not None:
entry_point = code.get_input_plugin_name()
entry_point = code.default_calc_job_plugin
if entry_point != self._entry_point:
raise click.BadParameter(
'the retrieved Code<{}> has plugin type "{}" while "{}" is required'.format(
Expand Down
6 changes: 3 additions & 3 deletions aiida/engine/daemon/execmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
from aiida.common.folders import SandboxFolder
from aiida.common.links import LinkType
from aiida.manage.configuration import get_config_option
from aiida.orm import CalcJobNode, Code, FolderData, Node, RemoteData, load_node
from aiida.orm import CalcJobNode, Code, FolderData, Node, PortableCode, RemoteData, load_node
from aiida.orm.utils.log import get_dblogger_extra
from aiida.repository.common import FileType
from aiida.schedulers.datastructures import JobState
Expand Down Expand Up @@ -174,7 +174,7 @@ def upload_calculation(
# Still, beware! The code file itself could be overwritten...
# But I checked for this earlier.
for code in input_codes:
if code.is_local():
if isinstance(code, PortableCode):
# Note: this will possibly overwrite files
for filename in code.base.repository.list_object_names():
# Note, once #2579 is implemented, use the `node.open` method instead of the named temporary file in
Expand All @@ -184,7 +184,7 @@ def upload_calculation(
handle.write(code.base.repository.get_object_content(filename, mode='rb'))
handle.flush()
transport.put(handle.name, filename)
transport.chmod(code.get_local_executable(), 0o755) # rwxr-xr-x
transport.chmod(code.filepath_executable, 0o755) # rwxr-xr-x

# local_copy_list is a list of tuples, each with (uuid, dest_path, rel_path)
# NOTE: validation of these lists are done inside calculation.presubmit()
Expand Down
22 changes: 11 additions & 11 deletions aiida/engine/processes/calcjobs/calcjob.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def define(cls, spec: CalcJobProcessSpec) -> None: # type: ignore[override]
# yapf: disable
super().define(spec)
spec.inputs.validator = validate_calc_job # type: ignore[assignment] # takes only PortNamespace not Port
spec.input('code', valid_type=orm.Code, required=False,
spec.input('code', valid_type=orm.AbstractCode, required=False,
help='The `Code` to use for this job. This input is required, unless the `remote_folder` input is '
'specified, which means an existing job is being imported and no code will actually be run.')
spec.input('remote_folder', valid_type=orm.RemoteData, required=False,
Expand Down Expand Up @@ -592,7 +592,7 @@ def presubmit(self, folder: Folder) -> CalcInfo:
from aiida.common.datastructures import CodeInfo, CodeRunMode
from aiida.common.exceptions import InputValidationError, InvalidOperation, PluginInternalError, ValidationError
from aiida.common.utils import validate_list_of_string_tuples
from aiida.orm import Code, Computer, load_node
from aiida.orm import AbstractCode, Code, Computer, InstalledCode, PortableCode, load_node
from aiida.schedulers.datastructures import JobTemplate, JobTemplateCodeInfo

inputs = self.node.base.links.get_incoming(link_type=LinkType.INPUT_CALC)
Expand All @@ -601,19 +601,19 @@ def presubmit(self, folder: Folder) -> CalcInfo:
raise InvalidOperation('calculation node is not stored.')

computer = self.node.computer
codes = [_ for _ in inputs.all_nodes() if isinstance(_, Code)]
codes = [_ for _ in inputs.all_nodes() if isinstance(_, AbstractCode)]

for code in codes:
if not code.can_run_on(computer):
if not code.can_run_on_computer(computer):
raise InputValidationError(
'The selected code {} for calculation {} cannot run on computer {}'.format(
code.pk, self.node.pk, computer.label
)
)

if code.is_local() and code.get_local_executable() in folder.get_content_list():
if isinstance(code, PortableCode) and str(code.filepath_executable) in folder.get_content_list():
raise PluginInternalError(
f'The plugin created a file {code.get_local_executable()} that is also the executable name!'
f'The plugin created a file {code.filepath_executable} that is also the executable name!'
)

calc_info = self.prepare_for_submission(folder)
Expand Down Expand Up @@ -663,12 +663,12 @@ def presubmit(self, folder: Folder) -> CalcInfo:
# would return None, in which case the join method would raise
# an exception
prepend_texts = [computer.get_prepend_text()] + \
[code.get_prepend_text() for code in codes] + \
[code.prepend_text for code in codes] + \
[calc_info.prepend_text, self.node.get_option('prepend_text')]
job_tmpl.prepend_text = '\n\n'.join(prepend_text for prepend_text in prepend_texts if prepend_text)

append_texts = [self.node.get_option('append_text'), calc_info.append_text] + \
[code.get_append_text() for code in codes] + \
[code.append_text for code in codes] + \
[computer.get_append_text()]
job_tmpl.append_text = '\n\n'.join(append_text for append_text in append_texts if append_text)

Expand Down Expand Up @@ -696,7 +696,7 @@ def presubmit(self, folder: Folder) -> CalcInfo:

if code_info.code_uuid is None:
raise PluginInternalError('CalcInfo should have the information of the code to be launched')
this_code = load_node(code_info.code_uuid, sub_classes=(Code,))
this_code = load_node(code_info.code_uuid, sub_classes=(Code, InstalledCode, PortableCode))

# To determine whether this code should be run with MPI enabled, we get the value that was set in the inputs
# of the entire process, which can then be overwritten by the value from the `CodeInfo`. This allows plugins
Expand All @@ -715,12 +715,12 @@ def presubmit(self, folder: Folder) -> CalcInfo:
else:
prepend_cmdline_params = []

cmdline_params = [this_code.get_execname()] + (code_info.cmdline_params or [])
cmdline_params = [str(this_code.get_executable())] + (code_info.cmdline_params or [])

tmpl_code_info = JobTemplateCodeInfo()
tmpl_code_info.prepend_cmdline_params = prepend_cmdline_params
tmpl_code_info.cmdline_params = cmdline_params
tmpl_code_info.use_double_quotes = [computer.get_use_double_quotes(), this_code.get_use_double_quotes()]
tmpl_code_info.use_double_quotes = [computer.get_use_double_quotes(), this_code.use_double_quotes]
tmpl_code_info.stdin_name = code_info.stdin_name
tmpl_code_info.stdout_name = code_info.stdout_name
tmpl_code_info.stderr_name = code_info.stderr_name
Expand Down
4 changes: 2 additions & 2 deletions aiida/engine/processes/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -713,8 +713,8 @@ def _setup_inputs(self) -> None:
continue

# Special exception: set computer if node is a remote Code and our node does not yet have a computer set
if isinstance(node, orm.Code) and not node.is_local() and not self.node.computer:
self.node.computer = node.get_remote_computer()
if isinstance(node, orm.InstalledCode) and not self.node.computer:
self.node.computer = node.computer

# Need this special case for tests that use ProcessNodes as classes
if isinstance(self.node, orm.CalculationNode):
Expand Down
23 changes: 16 additions & 7 deletions aiida/manage/tests/pytest_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,18 @@ def get_code(entry_point, executable, computer=aiida_localhost, label=None, prep
:rtype: :py:class:`aiida.orm.Code`
"""
from aiida.common import exceptions
from aiida.orm import Code, Computer, QueryBuilder
from aiida.orm import Computer, InstalledCode, QueryBuilder

if label is None:
label = executable

builder = QueryBuilder().append(Computer, filters={'uuid': computer.uuid}, tag='computer')
builder.append(Code, filters={'label': label, 'attributes.input_plugin': entry_point}, with_computer='computer')
builder.append(
InstalledCode, filters={
'label': label,
'attributes.input_plugin': entry_point
}, with_computer='computer'
)

try:
code = builder.one()[0]
Expand All @@ -218,15 +223,19 @@ def get_code(entry_point, executable, computer=aiida_localhost, label=None, prep
if not executable_path:
raise ValueError(f'The executable "{executable}" was not found in the $PATH.')

code = Code(input_plugin_name=entry_point, remote_computer_exec=[computer, executable_path])
code.label = label
code.description = label
code = InstalledCode(
label=label,
description=label,
default_calc_job_plugin=entry_point,
computer=computer,
filepath_executable=executable_path
)

if prepend_text is not None:
code.set_prepend_text(prepend_text)
code.prepend_text = prepend_text

if append_text is not None:
code.set_append_text(append_text)
code.append_text = append_text

return code.store()

Expand Down
Loading

0 comments on commit a2f6d79

Please sign in to comment.