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

provider: add a AWS Elemental MediaConvert provider #223

Merged
merged 8 commits into from
Jul 14, 2019

Conversation

zsiec
Copy link
Member

@zsiec zsiec commented Jul 12, 2019

@codecov-io
Copy link

codecov-io commented Jul 12, 2019

Codecov Report

Merging #223 into master will decrease coverage by 0.25%.
The diff coverage is 76.51%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #223      +/-   ##
==========================================
- Coverage   78.62%   78.36%   -0.26%     
==========================================
  Files          29       31       +2     
  Lines        2914     3310     +396     
==========================================
+ Hits         2291     2594     +303     
- Misses        372      446      +74     
- Partials      251      270      +19
Impacted Files Coverage Δ
config/config.go 100% <ø> (ø) ⬆️
provider/mediaconvert/preset_mapping.go 71.95% <71.95%> (ø)
provider/mediaconvert/mediaconvert.go 79.74% <79.74%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c4b037...aff937e. Read the comment docs.

@fsouza fsouza self-assigned this Jul 12, 2019
Copy link
Collaborator

@fsouza fsouza left a comment

Choose a reason for hiding this comment

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

Looks good overall, just some minor comments.

Nice job! :D

README.md Show resolved Hide resolved
provider/mediaconvert/mediaconvert.go Outdated Show resolved Hide resolved
provider/mediaconvert/mediaconvert.go Show resolved Hide resolved
provider/mediaconvert/mediaconvert.go Show resolved Hide resolved
provider/mediaconvert/mediaconvert_test.go Outdated Show resolved Hide resolved
provider/mediaconvert/mediaconvert_test.go Outdated Show resolved Hide resolved
provider/mediaconvert/mediaconvert_test.go Outdated Show resolved Hide resolved
provider/mediaconvert/mediaconvert_test.go Outdated Show resolved Hide resolved
provider/mediaconvert/mediaconvert_test.go Outdated Show resolved Hide resolved
provider/mediaconvert/preset_mapping.go Show resolved Hide resolved
Copy link
Collaborator

@fsouza fsouza left a comment

Choose a reason for hiding this comment

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

One last thing x)

provider/mediaconvert/testing.go Outdated Show resolved Hide resolved
zsiec added 5 commits July 13, 2019 16:28
We don't want this code to do anything inexpected.

By checking these values separately, if a config is incomplete and a key was provided but no secret (or the other way around) the static creds logic is not run and when run, nothing will error and default creds may be used unexpectedly. Let's make sure that if anything was provided for key or secret, we run the static credentials logic.
@fsouza fsouza merged commit 4ebb3f4 into video-dev:master Jul 14, 2019
@zsiec zsiec deleted the nytimes-mediaconvert branch July 15, 2019 00:17
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.

providers: add support for mediaconvert
3 participants