New module: Add amazon cloudfront distribution module. (cloud/amazon/cloudfront_distribution) #24292
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
ansibot
added
affects_2.4
|
The test The test The test The test The test The test The test |
ansibot
added
needs_revision
nitzmahone
removed
the
needs_triage
ansibot
removed
the
ci_verified
|
@Etherdaemon @Java1Guy @akazakov @alachaum @amir343 @bekelchik @bpennypacker @brandond @fiunchinho @j-carl @jarv @jmenga @joelthompson @jsdalton @linuxdynasty @loia @MichaelBaydoun @michaeljs1990 @minichate @mjschultz @mmochan @naslanidis @pjodouin @pwnall @RickMendes @rmorlok @ryansydnor @scottanderson42 @silviud @simplesteph @steynovich @tastychutney @tedder @timmahoney @TomBamford @whiter @willthames @wimnat @Zeekin As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
ansibot
added
community_review
|
@wilvk This PR contains |
ansibot
added
needs_revision
|
I don't see how this module can be idempotent as there's no state parameter. It's more procedural which is not really the ansible way. You should have a purge_tags option if you're supporting tags to decide what to do with existing tags that aren't defined in the playbook. |
|
@wilvk this PR contains the following merge comits: Please rebase your branch to remove these commits. |
ansibot
added
merge_commit
ansibot
added
community_review
|
bot_skip |
|
@s-hertel I've moved the See below for an example of the Shippable output: |
…, removed from legacy-files, fix for dependency on module in shared resource
|
ready_for_review |
|
@jctanner is this PR triggering review requests with the bot skip tag? I requested a review a week ago with no response.. |
|
@wilvk I'm sorry, I've seen your request, just have too many things on my plate. I'll get back to this as soon as possible. |
|
I'd like to get this PR back on track as this would be a valuable module. There needs to be a separation of the cloudfront_facts module from the cloudfront_distribution module. Most of the shared I'm not a huge fan of classes for modules (as in As such, my way forward would be:
It might be worth having a combined shared library for all cloudfront modules, as @wilvk, let me know if that seems like a reasonable proposal and, if so, if you have the time to do it - if not, I can start looking at the improvements based on your existing work. |
|
@willthames thanks for the feedback on this one - was a bit stuck on it. I'll have some time this weekend to look at it (still working on the ec2_ami tests stuff) otherwise feel free to start making the changes. |
|
Closing in favor of #31284 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
SUMMARY
This module allows the following cloudfront actions:
I saw this as a gap item as the
cloudfront_facts.pymodule already exists but notcloudfront.py.Each action has a set of valid parameters that an be used in conjunction. These are illustrated in the examples section.
This PR also renames the class for
cloudfront_facts.pyasCloudfrontFactsManagerasCloudfrontManageris more appropriate for this module.ISSUE TYPE
New Module Pull Request
COMPONENT NAME
./lib/ansible/modules/cloud/amazon/cloudfront.py
ANSIBLE VERSION
ADDITIONAL INFORMATION
This PR is migrated from PR #24151 as the branch and PR comment history was irreparable and has thus been closed.
comments from @willthames:
create_streaming_distributionanddelete_streaming_distributionand the other similarly named parameters are of concern - we should be stating the desired state ofstreaming_distribution(present or absent).This feels like it should probably be multiple modules.
The use of
tag_resourceanduntag_resourceis problematic - just provide atags:parameter and update tags accordinglycomments from @wilvk:
Thanks for the feedback @willthames ! I'll get onto those changes:
consolidate tag and untag into just
tags:use
statusparameter instead ofcreate_distributiondelete_distributionupdate_distributionduplicate_distributionvalidate_distributionetc.could possibly be multiple modules but not sure how it should be split - distribution and streaming_distribution have dependencies/common methods. Am open to suggestions, however..