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

feat: Add additional properties for Cloudwatch Schedule - Enabled, Name, Description #1006

Merged
merged 4 commits into from
Jul 16, 2019

Conversation

sambattalio
Copy link
Contributor

@sambattalio sambattalio commented Jul 1, 2019

Issue #, if available:
#934
Description of changes:
Added the ability to set the name, description, and enable state of Cloudwatch Schedules.

Example usage of new properties:

Type: Schedule
Properties:
    Schedule: 'rate(1 minute)'
    Name: test-schedule
    Description: test schedule
    Enabled: True

Description of how you validated changes:
Validated changes through testing transforms as described in the contributing guidelines
and by verifying with an appended test case.

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation
  • Verify transformed template deploys and application functions as expected
  • Add/update example to examples/2016-10-31

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sambattalio
Copy link
Contributor Author

sambattalio commented Jul 1, 2019

Wondering if someone can help me figure out why the travis-ci build is failing. It seems like the 7 tests that are failing have been failing from the time I forked the repo.

@sambattalio sambattalio changed the base branch from master to develop July 1, 2019 14:13
@sambattalio sambattalio force-pushed the feature/AddCloudwatchScheduleProps branch from c4b55b4 to 465ba2a Compare July 1, 2019 14:27
@sambattalio sambattalio changed the title feat: Add additional properties for Cloudwatch Schedule - Enabled, Name, Description (#934) feat: Add additional properties for Cloudwatch Schedule - Enabled, Name, Description Jul 4, 2019
@yan12125
Copy link
Contributor

yan12125 commented Jul 6, 2019

From the error message:

Unknown exception while processing rule E3002: 'AWS::Cognito::UserPool.Policies'

I guess it's related to a cfn-lint issue: aws-cloudformation/cfn-lint#1002.

@sambattalio
Copy link
Contributor Author

From the error message:

Unknown exception while processing rule E3002: 'AWS::Cognito::UserPool.Policies'

I guess it's related to a cfn-lint issue: aws-cloudformation/cfn-python-lint#1002.

@yan12125 that looked to be exactly the case. After triggering a rebuild on travis-ci after cfn-lint 22.2 released all the tests are passing! Thanks for the heads up on that issue. 👍

@brettstack
Copy link
Contributor

Thanks for the contribution! Could you add a test for this?

@codecov-io
Copy link

Codecov Report

Merging #1006 into develop will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop    #1006     +/-   ##
==========================================
+ Coverage    94.71%   94.82%   +0.1%     
==========================================
  Files           69       69             
  Lines         3160     3244     +84     
  Branches       606      632     +26     
==========================================
+ Hits          2993     3076     +83     
- Misses          85       87      +2     
+ Partials        82       81      -1
Impacted Files Coverage Δ
samtranslator/model/eventsources/push.py 88.6% <100%> (-0.25%) ⬇️
samtranslator/swagger/swagger.py 97.49% <0%> (-0.18%) ⬇️
samtranslator/model/apigateway.py 97.72% <0%> (ø) ⬆️
samtranslator/model/sam_resources.py 95.59% <0%> (ø) ⬆️
samtranslator/model/api/api_generator.py 94.92% <0%> (+1.34%) ⬆️

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 88df20c...005899f. Read the comment docs.

@sambattalio
Copy link
Contributor Author

Added an additional test for the schedule properties. Let me know if there is anything else I can do/fix for this!

@praneetap praneetap merged commit 33572b3 into aws:develop Jul 16, 2019
@dvorontsov
Copy link

"Enabled: True"

Why capital "T", when other event types are "Enabled: true"?

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.

6 participants