-
Notifications
You must be signed in to change notification settings - Fork 217
Conversation
Awesome! 👍 |
#### Type: route53* | ||
#### Type: rds_db_parameter_group | ||
|
||
Note that this type is currently only available to list via puppet |
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 not totally sure what this means. I think it is something like:
Note that currently, this type can only be listed via puppet resource
, but cannot be created by Puppet.
f555f4b
to
0bded84
Compare
@jbondpdx I've updated the PR with all the suggested changes, and I've rebased with master to include the other README updates. |
Awesome, thank you, @garethr . +1 |
def destroy | ||
Puppet.info("Deleting database #{name} in region #{resource[:region]}") | ||
rds = rds_client(resource[:region]) | ||
Puppet.info("Skip Final Snapshot: #{resource[:skip_final_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.
It would be nice to have a proper message saying what will or will not be done here, instead of just dumping the value.
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.
Agreed, fixed.
c67b307
to
36641b9
Compare
@DavidS I've updated this PR with suggestions and rebased against master. It would be useful to review sooner rather than later in order to make sure @zreichert has time to review in this sprint once it's been through CI. Cheers |
[:present, :creating, :available].include? @property_hash[:ensure] | ||
end | ||
|
||
end |
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.
When trying to create at rds_db_parameter_group, puppet errors badly:
Error: Could not set 'present' on ensure: undefined method `create' for Rds_db_parameter_group[name=test2]:Puppet::Type::Rds_db_parameter_group
It'd be great if we could add create and destroy methods that fail with useful error messages.
(also, missing newline at EOF)
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.
Actually, why is that even ensurable? I've removed that, much more sensible:
puppet resource rds_db_parameter_group garethr ensure=present region=sa-east-1
Error: Could not run: Invalid parameter ensure
I see that rds_db_parameter_group is currently quite limited. It'd be great to have at least one acceptance test for it though. Not the least because I do not understand this behaviour:
|
|
Fixed
The issue you're seeing with ensure=absent is that you always have to specify the region in the params, not just the ENV variable. This is on purpose, but I think we should change it so what you expected to work just works. See #117, although that work needs prioritising. |
(CLOUD-295) Add RDS resource support
This takes on the work from @petems in #161 and: