rose macro: suite level functionality #1884
Conversation
| @@ -947,7 +950,7 @@ def run_macros(config_map, meta_config, config_name, macro_names, | |||
| reporter=reporter) | |||
| if not rc and no_changes: | |||
| reporter(MacroFinishNothingEvent()) | |||
| sys.exit(rc) | |||
| return True if rc == 0 else False | |||
There was a problem hiding this comment.
You could just return rc == 0
|
Test battery pep8 checking item is failing: One comment above. @benfitzpatrick - please review 2 and confirm you're happy with these changes as resident rose macro expert. I've manually checked through a few examples I've got knocking around (both on CLI and in rose edit) but you're probably best placed to know any of the awkward cases that might cause issues. |
|
I believe this change should close #1498. We can raise a follow-on issue for the |
| config_file_path = os.path.join(conf_dir, rose.TOP_CONFIG_NAME) | ||
| if (os.path.exists(config_file_path) and | ||
| os.path.isfile(config_file_path)): | ||
| for _, subdirs, _ in os.walk(os.path.join(conf_dir, |
There was a problem hiding this comment.
I know it sounds weird, but there can be apps with rose-app.conf files a couple of levels deep within their own directory structure! It's probably easier to do the same as we do with optional configuration files above (glob.glob) and only look for SUB_CONFIG_DIR + "/*/" + SUB_CONFIG_NAME.
|
This is a good change that people have been wanting for a while. Two comments above, and also I guess that technically running |
Agreed, that sounds safest. |
|
| else: | ||
| # are we in an apps folder? | ||
| apps = [] | ||
| for _, subdirs, _ in os.walk(conf_dir): |
There was a problem hiding this comment.
Sorry, we could do with doing glob.glob here as well - or actually, it might be better to do what rose edit does and just go down the directory tree until we find a relevant configuration file. That avoids the problem of being stuck in e.g. etc/foo/bar/ suite subdirectory and running rose macro. rose edit in rose/config_editor/main.py does:
path = os.getcwd()
name_set = set([rose.SUB_CONFIG_NAME, rose.TOP_CONFIG_NAME])
while True:
if set(os.listdir(path)) & name_set:
break
path = os.path.dirname(path)
if path == os.path.dirname(path):
# We don't support suites located at the root!
break
if path != os.getcwd() and path != os.path.dirname(path):
os.chdir(path)Maybe we could pull this out into a new function and call that?
There was a problem hiding this comment.
Or we could do it in a later PR
oliver-sanders
force-pushed
the
1498.2
branch
2 times, most recently
from
8b8677e
to
2215925
Compare
|
@benfitzpatrick Changed logic a bit. |
|
OK, good - just needs |
|
@benfitzpatrick Fixed syntax issue. |
Closes the suite-level aspect of #1498
@arjclark Please Review
@benfitzpatrick Please Review