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

Feature/redshift #249

Merged
merged 9 commits into from
Feb 6, 2017
Merged

Feature/redshift #249

merged 9 commits into from
Feb 6, 2017

Conversation

nadlerjessie
Copy link
Contributor

@nadlerjessie nadlerjessie commented Jan 18, 2017

This PR adds support for checking Redshift limits, specifically manual snapshots and cluster subnet groups. The default limits are specified by Amazon here.

Before submitting pull requests, please see the
Development documentation
and specifically the Pull Request Guidelines.

IMPORTANT: Please take note of the below checklist, especially the first two items.

Pull Request Checklist

  • All pull requests should be against the develop branch, not master.
  • All pull requests must include the Contributor License Agreement (see below).
  • Code should conform to the Development Guidelines:
    • pep8 compliant with some exceptions (see pytest.ini)
    • 100% test coverage with pytest (with valid tests). If you have difficulty
      writing tests for the code, feel free to ask for help or submit the PR without tests.
    • Complete, correctly-formatted documentation for all classes, functions and methods.
    • documentation has been rebuilt with tox -e docs
    • Connections to the AWS services should only be made by the class's
      connect() and connect_resource() methods, inherited from
      awslimitchecker.connectable.Connectable
    • All modules should have (and use) module-level loggers.
    • Commit messages should be meaningful, and reference the Issue number
      if you're working on a GitHub issue (i.e. "issue #x - "). Please
      refrain from using the "fixes #x" notation unless you are sure that the
      the issue is fixed in that commit.
    • Git history is fully intact; please do not squash or rewrite history.

Contributor License Agreement

By submitting this work for inclusion in awslimitchecker, I agree to the following terms:

  • The contribution included in this request (and any subsequent revisions or versions of it)
    is being made under the same license as the awslimitchecker project (the Affero GPL v3,
    or any subsequent version of that license if adopted by awslimitchecker).
  • My contribution may perpetually be included in and distributed with awslimitchecker; submitting
    this pull request grants a perpetual, global, unlimited license for it to be used and distributed
    under the terms of awslimitchecker's license.
  • I have the legal power and rights to agree to these terms.

@jantman
Copy link
Owner

jantman commented Jan 18, 2017

@nadlerjessie Thanks so much for this. I just wanted to give you a heads-up that (1) I just cut a release this past weekend, so I don't think the next is coming out terribly soon, and (2) I'm pretty swamped with work this week, so it might be a few days before I really review this.

Also, it looks like this needs a rebase on develop, but the changes should be pretty minor.

I really, really appreciate contributions, especially when they're as thorough as this looks. I know there are some test failures, but I doubt they're terribly complicated. I'll review this as soon as I can; I'll try to this weekend if not before.

Thanks!

@codecov-io
Copy link

codecov-io commented Jan 18, 2017

Codecov Report

❗ No coverage uploaded for pull request base (develop@b8aea0f). Click here to learn what that means.

@@           Coverage Diff            @@
##             develop   #249   +/-   ##
========================================
  Coverage           ?   100%           
========================================
  Files              ?     23           
  Lines              ?   1683           
  Branches           ?    259           
========================================
  Hits               ?   1683           
  Misses             ?      0           
  Partials           ?      0
Impacted Files Coverage Δ
awslimitchecker/services/init.py 100% <100%> (ø)
awslimitchecker/services/redshift.py 100% <100%> (ø)

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 b8aea0f...c2f15fe. Read the comment docs.

@nadlerjessie
Copy link
Contributor Author

@jantman No worries on any of the timing issues, I appreciate the heads up. I have rebased against the develop branch and fixed the issues with the tests.

Thanks!

@jantman
Copy link
Owner

jantman commented Feb 6, 2017

Apologies for the delay, but thank you so much for the contribution! I'm merging this to develop now, and expect a release within the next few weeks.

@jantman jantman merged commit bb8c560 into jantman:develop Feb 6, 2017
jantman added a commit that referenced this pull request Feb 6, 2017
@jantman
Copy link
Owner

jantman commented Mar 11, 2017

This was just released in 0.8.0, which is now live on PyPI. Apologies for the long delay before this release.

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.

3 participants