-
-
Notifications
You must be signed in to change notification settings - Fork 353
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
Rds snapshot improvements #269
Conversation
module AWS | ||
class RDS | ||
class Real | ||
require 'fog/aws/parsers/rds/create_db_snapshot' |
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.
Is this supposed to re-use create parser, or should it have it's own?
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.
Your right it should have its own.
Looks like a great start, one comment inline and would be great to also see some tests more fleshed out. Thanks! |
'snapshotId' => String | ||
} | ||
|
||
Fog.mock! |
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.
I am unsure if this is required though i couldnt pass the test cases without it..throws a
Missing required arguments: aws_access_key_id, aws_secret_access_key (ArgumentError)
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.
I don't think this should be here (it would be set or not at a higher level). The test you mention is broken on master I'm pretty sure, so we can consider this good even without this. If you can remove this I think we should be good to merge the rest. Thanks!
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.
Ok. I have removed it. Thank you!
Looks good, thanks! |
Implemented API for ModifyDBSnapshotAttribute and CopyDBSnapshot