[feat] Extend allowed syntax for system/partitions/environment names and enforce strict syntax checking for extra_resources keys - #3725
Conversation
Signed-off-by: Vasileios Karakasis <vkarak@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3725 +/- ##
===========================================
- Coverage 91.92% 91.53% -0.40%
===========================================
Files 62 62
Lines 13808 13808
===========================================
- Hits 12693 12639 -54
- Misses 1115 1169 +54 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hello @vkarak, I'm reporting back from #3720. First of all, thanks for taking the time, it is really appreciated. I had some tests with my config. My original need was to allow version-like names for environments, i.e., I then tried adding
Just to be clear, I'm already happy with the current status, and if this is, in fact, an expected behaviour, well, sorry for the noise, and do not hesitate to just merge 🙃 |
|
@RiMillo This is expected behaviour because the |
|
Ah, you are right, of course. I did not think about that! Indeed it works when escaping Cheers! |
Summary
Broaden the
alphanum_ext_stringschema type so that system, partition, environment and feature names may:.) and plus signs (+), in addition to the already-allowed dash (-) and underscore (_).The
valid_systems/valid_prog_environsmini-language is updated in lockstep so that every name accepted by the configuration schema can also be targeted precisely in a test'svalid_systems/valid_prog_environs.This PR also enforces strict syntax in the
extra_resourceskeys as these are expanded in kwargs and if they are not valid identifiers, ReFrame will crash, e.g., here:reframe/reframe/core/pipeline.py
Line 2830 in 00ffd41
Replaces #3720
Closes #3645
Closes #3723
Implementation details
Schema (
reframe/schemas/config.json)alphanum_ext_stringpattern:^[a-zA-Z_](?:[a-zA-Z0-9_-])*$→^[a-zA-Z0-9_](?:[a-zA-Z0-9_.+-])*$.A name still may not start with
.,+or-; this invariant is what keeps the mini-language tokenizer unambiguous (a bare name can never be mistaken for a+feat/-feat/%key=valtoken).propertyNamespatterns ofsystems.partitions.extras,environments.extrasandenvironments.resourcesnow reference the shared#/defs/alphanum_stringdef (no behavioural change — the inline patterns were already identical).systems.partitions.resources[].nameis now validated againstalphanum_stringinstead of accepting an arbitrary string.Mini-language (
reframe/core/pipeline.py)_N(the name production used by_VALID_SYS_SYNTAX/_VALID_ENV_SYNTAX) gains+. Leading digits and.were already accepted by_N; with this change the schema pattern and_Nare equivalent for ASCII input.Parser (
reframe/core/runtime.py)_is_valid_part()/_is_valid_env()now split%key=valspecs withmaxsplit=1, so a value containing=(e.g.%key=a=b) no longer raises an uncaughtValueError.Docs (
docs/config_reference.rst)systems.name,systems.partitions.name,environments.name(the last one previously said nothing) and the twofeatureslists.^[a-zA-Z_][a-zA-Z0-9_]*$).versionchanged:: 4.11.0markers throughout.Grammar: before → after
sys.x:part.y0sys:1partg++,gcc-11.2+ompsys:gpu+mps,+gpu+mps.sys,+sys,-syssys x,a:b:c,a@b