Skip to content
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

Add support for main.yaml in a role instead of only supporting main.yml #356

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions lsr_role2collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,21 @@ def file_replace(path, find, replace, file_patterns):
with open(filepath, "w", encoding="utf-8") as f:
f.write(s)

def main_file_exists(src_path, role):
filenames = ["main.yml", "main.yaml"]
paths = [
os.path.join(src_path, "tasks"),
os.path.join(src_path, role, "tasks")
]

for path in paths:
for filename in filenames:
logging.debug(f"Testing if {os.path.join(path, filename)} is a file.")
if os.path.isfile(os.path.join(path, filename)):
logging.debug(f"{os.path.join(path, filename)} exists and is a file - setting src_path to {path}.")
src_path = os.path.join(path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting src_path here does not set src_path in the calling context. Maybe have this function return src_path if the main YAML file exists, or raise an error if the file does not exist?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, and in this case rename this function to get_main_file

return True
return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return False
raise FileNotFoundError(errno.ENOENT, f"Could not find {filenames[0]} or {filenames[1]} for {role} in {paths[0]} or {paths[1]}", "")


def copy_tree_with_replace(
src_path,
Expand Down Expand Up @@ -1338,12 +1353,8 @@ def parse_extra_mapping(mapping_str, namespace, collection, role):
src_owner = args.src_owner
if not src_owner:
src_owner = os.path.basename(src_path)
_tasks_main = src_path / "tasks/main.yml"
if not _tasks_main.exists():
src_path = src_path / role
_tasks_main = src_path / "tasks/main.yml"

if not _tasks_main.exists():
if not main_file_exists(src_path, role):
logging.error(
f"Neither {src_path} nor {src_path.parent} is a role top directory."
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace the if and the logging with just

    src_path = main_file_exists(src_path, role)

Expand Down