-
Notifications
You must be signed in to change notification settings - Fork 93
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
Infer run number & centralised reg parse #4285
Changes from 14 commits
fe9418b
2fbfda7
50cbe95
70673f1
007c3e2
1a9e119
135ab1a
7c1ce36
7c6721d
e26da36
7f6db31
653f52b
dc58221
57be420
ed0845f
4122314
faa9bcc
9b68a87
fc44f5d
d1d44c9
32f8f64
8fdec12
a932ce6
56cc652
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -20,13 +20,15 @@ | |||||||||||||||||
import re | ||||||||||||||||||
from copy import deepcopy | ||||||||||||||||||
from typing import ( | ||||||||||||||||||
Any, Dict, Iterable, List, Optional, Tuple, Union) | ||||||||||||||||||
Any, Dict, Iterable, List, Optional, Tuple, Union, overload) | ||||||||||||||||||
|
||||||||||||||||||
from cylc.flow.exceptions import PlatformLookupError | ||||||||||||||||||
from cylc.flow.cfgspec.glbl_cfg import glbl_cfg | ||||||||||||||||||
from cylc.flow.hostuserutil import is_remote_host | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
UNKNOWN_TASK = 'unknown task' | ||||||||||||||||||
|
||||||||||||||||||
FORBIDDEN_WITH_PLATFORM: Tuple[Tuple[str, str, List[Optional[str]]], ...] = ( | ||||||||||||||||||
('remote', 'host', ['localhost', None]), | ||||||||||||||||||
('job', 'batch system', [None]), | ||||||||||||||||||
|
@@ -38,6 +40,20 @@ | |||||||||||||||||
PLATFORM_REC_COMMAND = re.compile(r'(\$\()\s*(.*)\s*([)])$') | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
@overload | ||||||||||||||||||
def get_platform( | ||||||||||||||||||
task_conf: Union[str, None] = None, task_id: str = UNKNOWN_TASK | ||||||||||||||||||
) -> Dict[str, Any]: | ||||||||||||||||||
... | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
@overload | ||||||||||||||||||
def get_platform( | ||||||||||||||||||
task_conf: Dict[str, Any], task_id: str = UNKNOWN_TASK | ||||||||||||||||||
) -> Optional[Dict[str, Any]]: | ||||||||||||||||||
... | ||||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
# BACK COMPAT: get_platform | ||||||||||||||||||
# At Cylc 9 remove all Cylc7 upgrade logic. | ||||||||||||||||||
# from: | ||||||||||||||||||
|
@@ -48,7 +64,7 @@ | |||||||||||||||||
# Cylc9 | ||||||||||||||||||
def get_platform( | ||||||||||||||||||
task_conf: Union[str, Dict[str, Any], None] = None, | ||||||||||||||||||
task_id: str = 'unknown task' | ||||||||||||||||||
task_id: str = UNKNOWN_TASK | ||||||||||||||||||
) -> Optional[Dict[str, Any]]: | ||||||||||||||||||
"""Get a platform. | ||||||||||||||||||
|
@@ -432,7 +448,7 @@ def fail_if_platform_and_host_conflict(task_conf, task_name): | |||||||||||||||||
|
||||||||||||||||||
|
||||||||||||||||||
def get_platform_deprecated_settings( | ||||||||||||||||||
task_conf: Dict[str, Any], task_name: str = 'unknown task' | ||||||||||||||||||
task_conf: Dict[str, Any], task_name: str = UNKNOWN_TASK | ||||||||||||||||||
) -> List[str]: | ||||||||||||||||||
"""Return deprecated [runtime][<task_name>] settings that should be | ||||||||||||||||||
upgraded to platforms. | ||||||||||||||||||
|
@@ -548,5 +564,5 @@ def get_random_platform_for_install_target( | |||||||||||||||||
|
||||||||||||||||||
def get_localhost_install_target() -> str: | ||||||||||||||||||
"""Returns the install target of localhost platform""" | ||||||||||||||||||
localhost = platform_from_name() | ||||||||||||||||||
localhost = get_platform() | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wxtim can you check this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just changing it back to how it was before; both identical as can be seen from cylc-flow/cylc/flow/platforms.py Lines 85 to 87 in bc72520
The reason I had previously changed it to cylc-flow/cylc/flow/platforms.py Lines 43 to 47 in bc72520
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've just found out about a nasty omission in mypy's checking of |
||||||||||||||||||
return get_install_target_from_platform(localhost) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies if I've missed the explanation on this one, is there a reason why these are both no longer optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't explain; it's because they should always be set by
cylc-flow/cylc/flow/config.py
Line 599 in 135ab1a
cylc-flow/cylc/flow/config.py
Line 647 in 135ab1a
(
start_point
takes on value ofinitial_point
if not specified)This stops mypy errors complaining that
start_point
orinitial_point
may beNone
when trying to usePointBase
methods later.Only problem is, if they somehow don't end up getting set, we'd end up with
AttributeError
s, which won't get picked up by mypy. But in that case, we'd still getTypeError
s if they were set toNone
in__init__
like before, I guess. Not sure what the best practice is, but at least this silences mypy.