Skip to content

Commit

Permalink
Cut down on back-compat warnings, plus other tidying (#4943)
Browse files Browse the repository at this point in the history
Cut down on back-compat warnings, plus other tidying

* Tidy and clear up cryptic comments
* Fix test failures when symlink dirs set in global config
* Improve Cylc7 graph compat message
* Tidy `flow.cylc[scheduler]install` description
* Improve deprecation warning for runtime settings (The ones that should be replaced by platform)
* Cut down on back-compat warnings
* Improve implicit task err msg
* Do not suggest using `allow implicit tasks` setting when rose-suite.conf present in c7 back-compat mode
  • Loading branch information
MetRonnie authored Jun 30, 2022
1 parent 1dc8bf2 commit 34748a8
Show file tree
Hide file tree
Showing 15 changed files with 99 additions and 119 deletions.
18 changes: 8 additions & 10 deletions cylc/flow/cfgspec/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,16 +220,13 @@ def get_script_common_text(this: str, example: Optional[str] = None):
The following directories are installed by default:
* app
* bin
* etc
* lib
And include the server.key file (from the .service
directory), this is required for authentication.
* ``app/``
* ``bin/``
* ``etc/``
* ``lib/``
These should be located in the top level of your Cylc workflow,
i.e. the directory that contains your flow.cylc file.
i.e. the directory that contains your ``flow.cylc`` file.
Directories must have a trailing slash.
For example, to add the following items to your file installation:
Expand Down Expand Up @@ -1862,8 +1859,9 @@ def warn_about_depr_platform(cfg):
if depr:
msg = "\n".join(depr)
LOG.warning(
f'Task {task_name}: deprecated "host" and "batch system"'
f' use "platform".\n{msg}'
"deprecated settings found "
f"(please replace with [runtime][{task_name}]platform):"
f"\n{msg}"
)


Expand Down
35 changes: 14 additions & 21 deletions cylc/flow/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,6 @@ class WorkflowConfig:
CHECK_CIRCULAR_LIMIT = 100 # If no. tasks > this, don't check circular
VIS_N_POINTS = 3

CYLC7_GRAPH_COMPAT_MSG = (
"Cylc 7 graph compatibility: making success outputs 'required' (to"
" retain failed tasks in the pool) and pre-spawning graph children (to"
" replicate Cylc 7 stall behaviour). Please refer to documentation on"
" upgrading Cylc 7 graphs to Cylc 8."
)

def __init__(
self,
workflow: str,
Expand Down Expand Up @@ -629,7 +622,8 @@ def process_initial_cycle_point(self) -> None:
except IsodatetimeError as exc:
raise WorkflowConfigError(str(exc))
if orig_icp != icp:
# now/next()/prev() was used, need to store evaluated point in DB
# now/next()/previous() was used, need to store
# evaluated point in DB
self.options.icp = icp
self.initial_point = get_point(icp).standardise()
self.cfg['scheduling']['initial cycle point'] = str(self.initial_point)
Expand Down Expand Up @@ -802,20 +796,21 @@ def _check_implicit_tasks(self) -> None:
raise WorkflowConfigError(msg)

# Otherwise "[scheduler]allow implicit tasks" is not set
msg = (

# Allow implicit tasks in back-compat mode (unless rose-suite.conf
# present, to maintain compat with Rose 2019)
if (
cylc.flow.flags.cylc7_back_compat and
not Path(self.run_dir, 'rose-suite.conf').is_file()
):
LOG.debug(msg)
return

raise WorkflowConfigError(
f"{msg}\n"
"To allow implicit tasks, use "
f"'{WorkflowFiles.FLOW_FILE}[scheduler]allow implicit tasks'\n"
"See https://cylc.github.io/cylc-doc/latest/html/"
"7-to-8/summary.html#backward-compatibility"
f"'{WorkflowFiles.FLOW_FILE}[scheduler]allow implicit tasks'"
)
# Allow implicit tasks in Cylc 7 back-compat mode (but not if
# rose-suite.conf present, to maintain compat with Rose 2019)
if (
Path(self.run_dir, 'rose-suite.conf').is_file() or
not cylc.flow.flags.cylc7_back_compat
):
raise WorkflowConfigError(msg)

def _check_circular(self):
"""Check for circular dependence in graph."""
Expand Down Expand Up @@ -2010,8 +2005,6 @@ def load_graph(self):
sections.append((section, value))

# Parse and process each graph section.
if cylc.flow.flags.cylc7_back_compat:
LOG.warning(self.__class__.CYLC7_GRAPH_COMPAT_MSG)
task_triggers = {}
task_output_opt = {}
for section, graph in sections:
Expand Down
52 changes: 22 additions & 30 deletions cylc/flow/cycling/iso8601.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,11 +617,11 @@ def _get_old_anchor_step_recurrence(anchor, step, start_point):
return str(anchor_point) + "/" + str(step)


def ingest_time(value: str, now: Optional['TimePoint'] = None) -> str:
def ingest_time(value: str, now: Optional[str] = None) -> str:
"""Handle relative, truncated and prev/next cycle points.
Args:
value: The string containing the prev()/next() stuff.
value: The string containing the previous()/next() stuff.
now: A time point to use as the context for resolving the value.
"""
# remove extraneous whitespace from cycle point
Expand All @@ -635,7 +635,7 @@ def ingest_time(value: str, now: Optional['TimePoint'] = None) -> str:
(value.startswith("-") or value.startswith("+"))
and "P" not in value
)
# prev() or next()
# previous() or next()
is_prev_next = "next" in value or "previous" in value
# offset from now (±P...)
is_offset = value.startswith("P") or value.startswith("-P")
Expand All @@ -657,34 +657,30 @@ def ingest_time(value: str, now: Optional['TimePoint'] = None) -> str:
# missing date-time components off the front (e.g. 01T00)
is_truncated = timepoint.truncated

if not any((is_prev_next, is_offset, is_truncated)):
if not (is_prev_next or is_offset or is_truncated):
return value

if now is None:
now = parser.parse(get_current_time_string())
else:
now = parser.parse(now)
now = get_current_time_string()
now_point = parser.parse(now)

# correct for year in 'now' if year only,
# or year and time, specified in input
# TODO: Figure out why this correction is needed
# correct for year in 'now' if year is the only date unit specified -
# https://github.com/cylc/cylc-flow/issues/4805#issuecomment-1103928604
if re.search(r"\(-\d{2}[);T]", value):
now += Duration(years=1)

# correct for month in 'now' if year and month only,
# or year, month and time, specified in input
now_point += Duration(years=1)
# likewise correct for month if year and month are the only date units
elif re.search(r"\(-\d{4}[);T]", value):
now += Duration(months=1)
now_point += Duration(months=1)

# perform whatever transformation is required
offset = None
if is_prev_next:
cycle_point, offset = prev_next(value, now, parser)
cycle_point, offset = prev_next(value, now_point, parser)
elif is_offset:
cycle_point = now
cycle_point = now_point
offset = value
else: # is_truncated
cycle_point = now + timepoint
cycle_point = now_point + timepoint

if offset is not None:
# add/subtract offset duration to/from chosen timepoint
Expand All @@ -700,10 +696,10 @@ def ingest_time(value: str, now: Optional['TimePoint'] = None) -> str:
def prev_next(
value: str, now: 'TimePoint', parser: 'TimePointParser'
) -> Tuple['TimePoint', Optional[str]]:
"""Handle prev() and next() syntax.
"""Handle previous() and next() syntax.
Args:
value: The string containing the prev()/next() stuff.
value: The string containing the previous()/next() stuff.
now: A time point to use as the context for resolving the value.
parser: A time point parser.
Expand All @@ -713,7 +709,7 @@ def prev_next(
# are we in gregorian mode (or some other eccentric calendar
if CALENDAR.mode != Calendar.MODE_GREGORIAN:
raise CylcConfigError(
'prev()/next() syntax must be used with integer or gregorian'
'previous()/next() syntax must be used with integer or gregorian'
f' cycling modes ("{value}")'
)

Expand Down Expand Up @@ -759,8 +755,8 @@ def prev_next(

cycle_point = timepoints[my_diff.index(min(my_diff))]

# ensure truncated dates do not have
# time from 'now' included'
# ensure truncated dates do not have time from 'now' included' -
# https://github.com/metomi/isodatetime/issues/212
if 'T' not in value.split(')')[0]:
# NOTE: Strictly speaking we shouldn't forcefully mutate TimePoints
# in this way as they're meant to be immutable since
Expand All @@ -771,18 +767,14 @@ def prev_next(
cycle_point._hour_of_day = 0
cycle_point._minute_of_hour = 0
cycle_point._second_of_minute = 0

# ensure month and day from 'now' are not included
# likewise ensure month and day from 'now' are not included
# where they did not appear in the truncated datetime
# NOTE: this may break when the order of tick over
# for time point is reversed!!!
# https://github.com/metomi/isodatetime/pull/101
# case 1 - year only
if re.search(r"\(-\d{2}[);T]", value):
# case 1 - year only
cycle_point._month_of_year = 1
cycle_point._day_of_month = 1
# case 2 - month only or year and month
elif re.search(r"\(-(-\d{2}|\d{4})[;T)]", value):
# case 2 - month only or year and month
cycle_point._day_of_month = 1

return cycle_point, offset
Expand Down
13 changes: 1 addition & 12 deletions cylc/flow/workflow_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,24 +338,13 @@ class RemoteCleanQueueTuple(NamedTuple):
* ssh -n "%(host)s" kill %(pid)s # final brute force!
"""

SUITERC_DEPR_MSG = (
"Backward compatibility mode ON for CYLC 7"
f" '{WorkflowFiles.SUITE_RC}' config files."
" When ready to upgrade, rename the file to"
f" {WorkflowFiles.FLOW_FILE} then address "
"any resulting validation errors and warnings."
)
SUITERC_DEPR_MSG = "Backward compatibility mode ON"

NO_FLOW_FILE_MSG = (
f"No {WorkflowFiles.FLOW_FILE} or {WorkflowFiles.SUITE_RC} "
"in {}"
)

REG_CLASH_MSG = (
"The specified reg could refer to ./{0} or ~/cylc-run/{1}. "
"This command will use ./{0}."
)

NESTED_DIRS_MSG = (
"Nested {dir_type} directories not allowed - cannot install workflow"
" in '{dest}' as '{existing}' is already a valid {dir_type} directory."
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/cylc-clean/02-targeted.t
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ create_test_global_config "" "
log = ${TEST_DIR}/${SYM_NAME}/other
share = ${TEST_DIR}/${SYM_NAME}/other
work = ${TEST_DIR}/${SYM_NAME}/other
# Need to override any symlink dirs set in global-tests.cylc:
# Need to override any symlink dirs set in global.cylc:
share/cycle =
"
install_workflow "${TEST_NAME_BASE}" basic-workflow
Expand Down
12 changes: 12 additions & 0 deletions tests/functional/cylc-install/03-file-transfer.t
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ if ! command -v 'tree' >'/dev/null'; then
fi
set_test_number 6

# Need to override any symlink dirs set in global.cylc:
create_test_global_config "" "
[install]
[[symlink dirs]]
[[[localhost]]]
run =
log =
work =
share =
share/cycle =
"

# Test cylc install copies files to run dir successfully.
TEST_NAME="${TEST_NAME_BASE}-basic"
make_rnd_workflow
Expand Down
12 changes: 12 additions & 0 deletions tests/functional/cylc-reinstall/01-file-transfer.t
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ if ! command -v 'tree' >'/dev/null'; then
fi
set_test_number 9

# Need to override any symlink dirs set in global.cylc:
create_test_global_config "" "
[install]
[[symlink dirs]]
[[[localhost]]]
run =
log =
work =
share =
share/cycle =
"

# Test cylc install copies files to run dir successfully.
TEST_NAME="${TEST_NAME_BASE}-basic"
make_rnd_workflow
Expand Down
6 changes: 1 addition & 5 deletions tests/functional/optional-outputs/03-c7backcompat.t
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
# sufficient to check the resulting validation and run time behaviour.

. "$(dirname "$0")/test_header"
set_test_number 6
set_test_number 5

install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"

Expand All @@ -44,10 +44,6 @@ DEPR_MSG_1=$(python -c \
'from cylc.flow.workflow_files import SUITERC_DEPR_MSG; print(SUITERC_DEPR_MSG)')
grep_ok "${DEPR_MSG_1}" "${TEST_NAME}.stderr"

DEPR_MSG_2=$(python -c \
'from cylc.flow.config import WorkflowConfig as cfg; print(cfg.CYLC7_GRAPH_COMPAT_MSG);')
grep_ok "${DEPR_MSG_2}" "${TEST_NAME}.stderr"

# And it should run without stalling with an incomplete task.
workflow_run_ok "${TEST_NAME_BASE}-run" \
cylc play -n --reference-test --debug "${WORKFLOW_NAME}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

# Cylc 7 stall backward compatibility, single parent case
# Cylc 7 stall backward compatibility, single parent case

. "$(dirname "$0")/test_header"
set_test_number 8
set_test_number 7

install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"

Expand All @@ -30,10 +30,6 @@ DEPR_MSG_1=$(python -c \
'from cylc.flow.workflow_files import SUITERC_DEPR_MSG; print(SUITERC_DEPR_MSG)')
grep_ok "${DEPR_MSG_1}" "${TEST_NAME}.stderr"

DEPR_MSG_2=$(python -c \
'from cylc.flow.config import WorkflowConfig as cfg; print(cfg.CYLC7_GRAPH_COMPAT_MSG);')
grep_ok "${DEPR_MSG_2}" "${TEST_NAME}.stderr"

# Should stall and abort with an unsatisfied prerequisite.
workflow_run_fail "${TEST_NAME_BASE}-run" \
cylc play -n --reference-test --debug "${WORKFLOW_NAME}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

# Cylc 7 stall backward compatibility, multi-parent case
# Cylc 7 stall backward compatibility, multi-parent case

. "$(dirname "$0")/test_header"
set_test_number 8
set_test_number 7

install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"

Expand All @@ -30,10 +30,6 @@ DEPR_MSG_1=$(python -c \
'from cylc.flow.workflow_files import SUITERC_DEPR_MSG; print(SUITERC_DEPR_MSG)')
grep_ok "${DEPR_MSG_1}" "${TEST_NAME}.stderr"

DEPR_MSG_2=$(python -c \
'from cylc.flow.config import WorkflowConfig as cfg; print(cfg.CYLC7_GRAPH_COMPAT_MSG);')
grep_ok "${DEPR_MSG_2}" "${TEST_NAME}.stderr"

# Should stall and abort with an unsatisfied prerequisite.
workflow_run_fail "${TEST_NAME_BASE}-run" \
cylc play -n --reference-test --debug "${WORKFLOW_NAME}"
Expand Down
6 changes: 1 addition & 5 deletions tests/functional/optional-outputs/06-c7backcompat-family.t
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# Cylc 7 stall backward compatibility, complex family case.

. "$(dirname "$0")/test_header"
set_test_number 13
set_test_number 12

install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"

Expand All @@ -30,10 +30,6 @@ DEPR_MSG_1=$(python -c \
'from cylc.flow.workflow_files import SUITERC_DEPR_MSG; print(SUITERC_DEPR_MSG)')
grep_ok "${DEPR_MSG_1}" "${TEST_NAME}.stderr"

DEPR_MSG_2=$(python -c \
'from cylc.flow.config import WorkflowConfig as cfg; print(cfg.CYLC7_GRAPH_COMPAT_MSG);')
grep_ok "${DEPR_MSG_2}" "${TEST_NAME}.stderr"

# Should stall and abort with unsatisfied "stall" tasks.
workflow_run_fail "${TEST_NAME_BASE}-run" \
cylc play -n --debug "${WORKFLOW_NAME}"
Expand Down
1 change: 0 additions & 1 deletion tests/functional/validate/37-special-implicit-task.t
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,5 @@ cmp_ok "${TEST_NAME_BASE}.stderr" << '__ERR__'
WorkflowConfigError: implicit tasks detected (no entry under [runtime]):
* foo
To allow implicit tasks, use 'flow.cylc[scheduler]allow implicit tasks'
See https://cylc.github.io/cylc-doc/latest/html/7-to-8/summary.html#backward-compatibility
__ERR__
exit
Loading

0 comments on commit 34748a8

Please sign in to comment.