-
Notifications
You must be signed in to change notification settings - Fork 607
adds volume configuration options #982
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tthebst This is awesome, great job! It's really great how you were able to figure out our conventions and where to put different parts of the code, that must have taken some additional effort, so thank you!
I added some comments, mostly minor. Let me know if you'd like to discuss any of them.
@tthebst Thanks for addressing my comments! I merged master in, and made a few small updates to the branch. I tried creating a cluster with I found this error message in my autoscaling group activity history:
Here's a screenshot: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks again for your help on this!
Adding @vishalbollu for a quick review, and then I'll merge it in
@deliahu Sorry I didn't notice that there was a 50:1 ratio restriction |
closes #592
checklist:
make test
andmake lint
summary.md
(view in gitbook after merging)