Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 general content-view management role #1177
add general content-view management role #1177
Changes from 3 commits
65991a9
4b5f0df
7c68eaf
e461cc4
3f89713
4c44afc
7a476b9
2c27d1e
d5949be
49ea232
178a41a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use the dedicated info module now: https://github.com/theforeman/foreman-ansible-modules/blob/develop/plugins/modules/content_view_info.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to have roles that utilize modules that aren't even in a main version yet? By using the existing format, this role will be backwards compatible with older versions of the collection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chiming in here, I think it does make sense. The primary way we expect users would consume this role would be to install the collection.
If they install the development branch from git they would have access to all of the latest modules. Of course it is more likely that they install one of the released versions from ansible galaxy, or a downstream RPM packaged version.... in each case still by the time this role is included in any of those releases, so too would be all modules currently present at development time.
Is this contrary to your expectation, @bmarlow ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wbclark I think its fair to assume people are going to get this by way of the collection so no need to worry there.
I guess my statement is more that I don't think it makes sense to build a role against a module that is in development as changes that happen to that module could potentially break the role.
As it sits (and based on the way that roles are consumed/delivered), I would think we wouldn't care which module got used as long as it worked (unless there was a significant performance benefit or depreciation issue) due to the fact that the user shouldn't really be modifying the role, but rather changing the variables that the role digests.
I haven't had a chance to use the new module yet, however my biggest concern would be that the data returned isn't structured the same way (maybe it is?) and it would require more significant refactoring (and lets face it, the data returned isn't the most easily parsable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a dedicated module for this as well in the next few days: #1169
Could be good feedback before merging it whether it handles your use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not fully understand this logic. Or the circumstances under which it may occur.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If your content-view's highest version number is 22, and that is also where your life-cycle environment is then we need to publish a new version prior to promoting- since you cannot promote to version 22+1 if version 23 doesn't exist.