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

BZ#1974314 - make cv cleanup work with keep=0 #1255

Merged
merged 2 commits into from Jul 12, 2021

Conversation

evgeni
Copy link
Member

@evgeni evgeni commented Jun 29, 2021

No description provided.

@evgeni
Copy link
Member Author

evgeni commented Jun 29, 2021

🍏

@@ -17,4 +17,4 @@
cv_name: "{{ cv.name }}"
cv_versions: "{{ (versions.resources | rejectattr('environments') | rejectattr('composite_content_view_ids') |
rejectattr('published_in_composite_content_view_ids') | map(attribute='version') | map('float') | sort |
map('string') | list )[:-foreman_content_view_version_cleanup_keep] }}"
map('string') | reverse | list )[foreman_content_view_version_cleanup_keep:] }}"
Copy link
Member Author

Choose a reason for hiding this comment

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

if you're curious what happen(s|ed) here:

alist[:-5] gives all but the last 5 entries of the list, while alist[:-0] is the same as alist[:0] which is sadly [].

now in normal python, I'd probably just do something like alist[:len(alist)-n], but given alist is really a super long thing we generate on the fly, and I really hate using set_fact, I didn't.

another (clever) solution would be to use alist[:-n or None], as -n or None evaluates to None for n=0, and then alist[:None] becomes alist[:] which is alist (well, technically a copy of it, but who cares). and I don't like clever too much.

now, given that we don't care about the order the CVs are removed, we can reverse the list, and start slicing at the beginning, where using 0 works just fine. still a bit clever, but less than the above, imho.

and because users, I've added a guard to disallow keep to be < 0.

@ehelms ehelms merged commit 7d8bb04 into theforeman:develop Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants