Platforms.eval platform cmd

[wxtim]

main response to #3672

Synopsis

As per the deprecation examples in the platforms proposal Cylc 8 is to be able to evaluate either:

• [TASK]platform = $(some-shell-script.sh)
• [TASK][remote]host = $(some-shell-script.sh)
  In a similar way at job submission time.

This PR attempts to create this behavior.

Work undertaken

• Modified the logic to work with an example suite.
• Written functional tests for each of the main scenarios this logic should handle:
  • Not interfering with platform = name and host = name.
  • Correctly expanding subshell host and platform settings.
  • Throwing a wobbly (raise Exception) if one tries to set both host and platform before attempting to expand subshells to avoid unnecessary calls to remote_host_select.

Follow on work not undertaken

• No attempt to update nomenclature of items in remote_host_select - should be straightforward, but would confuse the issue.
• No attempt to stop using backticks for platform.

[wxtim]

Unrelated, but not worth a full, separate PR.

[oliver-sanders]

Looks fine, testing to follow.

One legitimate test failure - tests/f/job-submission/18-platform_select.t

[wxtim]

tests/f/job-submission/18-platform_select.t

My fault - Dave suggested that I was raising the wrong error, so I changed the error I raised...

[hjoliver]

One test failing - fixed by #3806?

[wxtim]

One test failing - fixed by #3806?

Looks very much like it. 👍🏼

Can I request re-review from @oliver-sanders and @hjoliver

[oliver-sanders]

Closes #3672?

[oliver-sanders]

Looks good, testing I spotted some unexpected behaviour:

A non existent platform will result in a PlatformLookupError being raised which causes the suite to shutdown, I would have expected a submit failure:

platform = $(echo 'elephant')

2020-09-15T12:54:30+01:00 ERROR - PlatformLookupError: No matching platform "elephant" found

There seems to be some unintended pattern matching going on, for instance localhost-xyz-mess seems to get evaluated as localhost rather than resulting in a submit failure:

platform = $(echo 'localhost-xyz-mess')

2020-09-15T12:53:40+01:00 DEBUG - for task a.1: platform = $(echo 'localhost-xyz-mess') evaluated as localhost

[wxtim]

This should be faster since in most cases the try was going to fail.

[wxtim]

A non existent platform will result in a PlatformLookupError being raised which causes the suite to shutdown, I would have expected a submit failure:

I think I've fixed this in commit 3175fee

There seems to be some unintended pattern matching going on, for instance localhost-xyz-mess seems to get evaluated as localhost rather than resulting in a submit failure:

I think I have fixed this in commit e6838a6

[oliver-sanders]

Getting a submit failure now for platform=$(echo elephant) 👍. Sadly also getting this (harmless) traceback:

2020-09-15T15:55:08+01:00 ERROR - [Errno 2] No such file or directory:
	'/Users/oliver/cylc-run/foo/log/job/1/foo/01/job-activity.log'
	Traceback (most recent call last):
	  File "/Users/oliver/cylc-flow/cylc/flow/task_job_mgr.py", line 865, in
	_prep_submit_task_job
	    platform = get_platform(rtconfig)
	  File "/Users/oliver/cylc-flow/cylc/flow/platforms.py", line 82, in get_platform
	    output = platform_from_name(task_conf['platform'])
	  File "/Users/oliver/cylc-flow/cylc/flow/platforms.py", line 184, in
	platform_from_name
	    f"No matching platform \"{platform_name}\" found")
	cylc.flow.exceptions.PlatformLookupError: No matching platform "elephant" found
	During handling of the above exception, another exception occurred:
	Traceback (most recent call last):
	  File "/Users/oliver/cylc-flow/cylc/flow/task_events_mgr.py", line 87, in
	log_task_job_activity
	    with open(os.path.expandvars(job_activity_log), "ab") as handle:
	FileNotFoundError: [Errno 2] No such file or directory: '/Users/oliver/cylc-
	run/foo/log/job/1/foo/01/job-activity.log'

I think the old logic may have created the job activity log on failure to avoid this?

[wxtim]

Yes, @oliver-sanders pointed out that at some point I had deleted the line self._create_job_log_path(suite, itask) in task_job_mgr which is required to create the job-activity.log

[oliver-sanders]

All good I think.

The log messages contain a bit of duplicated info and could do with being tidied up a bit:

DEBUG - ['bash', '-c', 'echo
	localhost']
DEBUG - [remote-host-select cmd] bash -c
	'echo localhost'
	[remote-host-select ret_code] 0
	[remote-host-select out] localhost
DEBUG - for task foo.1: platform =
	$(echo localhost) evaluated as localhost

One message should suffice for this case, note the second log message comes from the subprocpool and contains all the required info.

DEBUG - Traceback (most recent call
	last):
	  File "/Users/oliver/cylc-flow/cylc/flow/task_job_mgr.py", line 866, in
	_prep_submit_task_job
	    platform = get_platform(rtconfig)
	  File "/Users/oliver/cylc-flow/cylc/flow/platforms.py", line 82, in get_platform
	    output = platform_from_name(task_conf['platform'])
	  File "/Users/oliver/cylc-flow/cylc/flow/platforms.py", line 184, in
	platform_from_name
	    f"No matching platform \"{platform_name}\" found")
	cylc.flow.exceptions.PlatformLookupError: No matching platform "elephant" found

ERROR - No matching platform "elephant" found
ERROR - [jobs-submit cmd] (platform not defined)
	[jobs-submit ret_code] 1
	[jobs-submit err] No matching platform "elephant" found

One message should suffice for this case, the traceback isn't helpful here as we already know where the exception was raised from.

[hjoliver]

Looks good, thanks @wxtim 👍

[hjoliver]

@wxtim - can you address the log duplication as a follow-up?