rewrite deploy.py so differently names roles and tags actually run#557
rewrite deploy.py so differently names roles and tags actually run#557KoenDR06 wants to merge 2 commits into
Conversation
| roles = roles[roles.index(from_playbook) :] | ||
| from_until = True | ||
| # ... we will filter this | ||
| role_data = [it for it in all_role_data] |
| ) | ||
| @click.option("--tags", "--roles", help="Roles to execute") | ||
| @click.option("--roles", help="Roles to execute") | ||
| @click.option("--skip", help="Roles to skip") |
There was a problem hiding this comment.
I would rename this to exclude, since --from seems ambigious with --skip
There was a problem hiding this comment.
Lol my original idea was skip but I now prefer exclude
| from_until = True | ||
| # ... we will filter this | ||
| role_data = [it for it in all_role_data] | ||
| all_roles = [it["role"] for it in all_role_data] |
There was a problem hiding this comment.
perhaps use map()? More verbose but more intuitive
| @click.option("--force", is_flag=True, default=False, help="Override checks") | ||
| @click.option( | ||
| "--from", | ||
| "from_playbook", |
There was a problem hiding this comment.
I now realise someone confused playbooks with roles, and as such from_playbook is a wrong synonym and should be removed. Same as until_playbook. from_playbook (used later) should be renamed to from_role
|
|
||
| ### Filter out all roles that fall outside specified range | ||
| i = 0 | ||
| if from_playbook is not None and from_playbook in all_roles: |
There was a problem hiding this comment.
So please rename from_playbook to from_role
| if from_playbook is not None and from_playbook in all_roles: | ||
| i = all_roles.index(from_playbook) | ||
| role_data = role_data[i:] | ||
| if until_playbook is not None and until_playbook in all_roles: | ||
| # We have to subtract i to correct for offset | ||
| i = all_roles.index(until_playbook) - i | ||
| if i < 0: | ||
| raise IndexError("`--until` role must be after the `--from` role") | ||
| role_data = role_data[: i + 1] | ||
|
|
||
| ### If we didn't specify from or until, clear data | ||
| if from_playbook is None and until_playbook is None: | ||
| role_data = [] |
There was a problem hiding this comment.
What if it is not in all_roles? These if statements are nonexhaustive thus creating edge cases where the code continues but the argument is incorrect.
Or to put it more concretely, if I say --from kaas all roles are used but I indended --from koala and I dont get an error so I assume everything starts from koala
| if roles is not None: | ||
| for role in roles.split(","): | ||
| role = role.strip() | ||
| role = next(filter(lambda x: x["role"] == role, all_role_data)) | ||
| role_data.append(role) | ||
|
|
||
| ### Exclude roles from --skip | ||
| if skip is not None: | ||
| for role in skip.split(","): | ||
| role = role.strip() | ||
| role_data = list(filter(lambda x: x["role"] != role, role_data)) | ||
|
|
There was a problem hiding this comment.
What if role is not in the list? Please check beforehand if all mentioned items are in the list of known roles
| if from_playbook is None and until_playbook is None: | ||
| role_data = [] |
There was a problem hiding this comment.
Why do we reset the list to empty? Shouldn't we use all roles, and as such just keep it as is?
3 participants
Also closes (#481)