-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
providers/aws: Update ElasticTranscoderPreset to have default for MaxFrameRate #13422
Conversation
@catsby as this now has a default, and is marked as ForceNew: true, will this cause any issues for some users? Do we need a State Migration for those who don't have it set? |
@stack72 The API defaults it to |
@catsby nice :) LGTM then |
@catsby This broke #9847 and undid #11340, I can no longer set a preset with a fixed frame rate in 0.9.3, this was the reason the default was removed in the first place. I now get:
|
I also ran into this problem and can't set a fixed frame rate. Is the fix as simple as reverting this single line? I can't tell since there's not really any context here explaining what the purpose of this PR was. Why should terraform ever have its own opinion on a default value if AWS specifically allows that value to be omitted? |
@dcosson this is now tracked as hashicorp/terraform-provider-aws#695 will address this issue. I think the only reason it got added by terraform as it made the tests easier. See #13638 for some of the back story. |
Got it, thanks for updating with the more in-depth comment. |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Updates
video.max_frame_rate
attribute to supply a default of30
, matching our Docs and AWS docs:Fixes test
TestAccAWSElasticTranscoderPreset_basic