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

Add generate-pipelines command #277

Merged
merged 3 commits into from
Apr 4, 2017
Merged

Add generate-pipelines command #277

merged 3 commits into from
Apr 4, 2017

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Apr 3, 2017

This will create a cfn template for a starter CD pipeline
based on your chalice app. I see this expanding over
time to allow more granular choices about the initial pipeline,
but for now a user will have to modify the generated template
before deploying.

NOTE: to test I had to change the sudo pip install chalice line to point to a copy that contained the package command. This won't be an issue once the package command is released on pypi.

jamesls added 2 commits April 4, 2017 09:11
This will create a cfn template for a starter CD pipeline
based on your chalice app.  I see this expanding over
time to allow more granular choices about the initial pipeline,
but for now a user will have to modify the generated template
before deploying.
@codecov-io
Copy link

codecov-io commented Apr 3, 2017

Codecov Report

Merging #277 into master will increase coverage by 0.57%.
The diff coverage is 98.8%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #277      +/-   ##
==========================================
+ Coverage   86.55%   87.13%   +0.57%     
==========================================
  Files          18       19       +1     
  Lines        1696     1780      +84     
  Branches      207      208       +1     
==========================================
+ Hits         1468     1551      +83     
- Misses        175      176       +1     
  Partials       53       53
Impacted Files Coverage Δ
chalice/cli/__init__.py 76.08% <100%> (+1.22%) ⬆️
chalice/constants.py 100% <100%> (ø) ⬆️
chalice/pipeline.py 98.63% <98.63%> (ø)

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 490952d...1b1d8fc. Read the comment docs.

Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks fine. Looks like a lot of hard coding of templates. I suspect as well many users will want to modify the default template as well. What would the process look like for users want to update the default template? Documentation would be nice.

]
}

CODEPIPELINE_POLICY = {
Copy link
Contributor

Choose a reason for hiding this comment

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

So is this the minimal amount of permissions needed to get codepipeline working? Seems a little weird that there is a good amount of services included in it that don't really need to be like beanstalk and opsworks

Copy link
Member Author

@jamesls jamesls Apr 4, 2017

Choose a reason for hiding this comment

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

That's from the console setup, yeah.

EDIT: I can also take a pass at paring these down. I'm a little hesitant to change the defaults from the console, but it should be possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. I am fine keeping it as it. It just pointed it out as it looked weird given you really only used half of those services and normally for IAM policies you keep it as minimal as possible.

@jamesls
Copy link
Member Author

jamesls commented Apr 4, 2017

The process isn't really defined at this point. It will really depend on the types of changes people want made. If anything I would expect more parameterization to happen, i.e github instead of codecommit, more stages, etc. Right now you just have to modify the template manually, but eventually we'd expose CLI options for these. Or at least that's what I had in mind.

@kyleknap
Copy link
Contributor

kyleknap commented Apr 4, 2017

@jamesls I guess I am wondering are you following up with documentation on how you can use the generate-pipeline command? It would be nice to at least have some sort of guidance on how to use these since it is not really defined as of now. It definitely took me some time to figure out how the workflow it would work.

@jamesls
Copy link
Member Author

jamesls commented Apr 4, 2017

@kyleknap How about this? I can update the PR to add docs for the generate-pipeline command so you can run chalice generate-pipeline --help to get more info on what this command does.

@kyleknap
Copy link
Contributor

kyleknap commented Apr 4, 2017

@jamesls sounds good to me.

Note that I also had to add an ignore rule for pydocstyle:

 D301: Use r""" if any backslashes in a docstring

The backslash style is required for click to preserve
newlines (http://click.pocoo.org/5/documentation/).
@jamesls
Copy link
Member Author

jamesls commented Apr 4, 2017

Cool added docs.

Copy link
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Nice! That definitely helps.

@jamesls jamesls merged commit 1b1d8fc into aws:master Apr 4, 2017
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.

4 participants