WIP: database_user and database refactoring #248
Conversation
| @@ -9,6 +9,8 @@ group :development, :test do | |||
| gem 'rspec-system-serverspec', :require => false | |||
| gem 'serverspec', :require => false | |||
| gem 'puppet-lint', :require => false | |||
| gem 'pry', :require => false | |||
There was a problem hiding this comment.
Actually it's in the development section... why not.
|
Could we do one PR for the renaming, and other PRs for the feature-adds? Just so the history is a little more clear. |
|
I could try and seperate them out but it would be tricky at this point. Not impossible, I'm just terrible at git so I have to look it up. |
|
Is there a reason for the rename? Its going to break a lot of modules.. |
|
It is going to break other modules but I think that's something we're going to just accept and move past. They were overly broad as it stands and absolutely unsuited to be generic types/providers. The long term plan is that we'll have mysql__, postgres__ types (and possibly others, sqlite, mongo etc) and then a new generic set of database_* types that call out to the other types for generic behavior that we can generalize across database providers. These were badly named from the start and I'd rather just bite the bullet on the pain of remaining them. I mean database_user{} is completely defined in terms of what mysql can do and any attempt to make them generic would remove half the features we support in the mysql types. Hopefully this longwinded ramble made sense. We can always try and improve the deprecation warnings to actually try and call the new names where possible, I just thought that was more risky than informing users. For the most part it's going to be a s/database/mysql_database/g for users so fixing modules won't take long. |
|
Wouldn't it be better to leave the old ones and add deprecation warnings. The new ones and old ones can live side by side for a while unless I'm mistaken? This is a fairly popular module and people are slow to upgrade. |
|
That could be done but the general idea is we bump the major version and if people don't want to deal with breaking changes they continue using a bugfix branch of the older version. I dunno, we haven't clearly defined exactly what our backwards compatibility is. I'm pretty OK with the idea of just keeping the old ones and providing an additional warning to each for a major version or so but I'm not OK with the idea of constantly trying to deal with never changing backwards functionality between major versions. I think I tend to be less conservative and more "Well, if you jump a major version fix your stuff", otherwise we stagnant. |
|
The problem is when you have two external modules that depend on different versions of the mysql module. |
|
|
Add collate as a new managable parameter, and extend self.instances to add in all parameters when checking existing databases. It also adds self.prefetch in order to speed up Puppet runs. Provider is also switched to using mk_resource_methods to generate all the resource readers, and exists? and other methods now use the property_hash where appropriate. Tests rewritten to handle changes and extend code coverage.
This work adds max_connections_per_hour, max_queries_per_hour, and max_updates_per_hour support to the provider and extends self.instances to add in the new parameters when checking existing users. It also adds self.prefetch in order to speed up Puppet runs. Provider is also switched to using mk_resource_methods to generate all the resource readers, and exists? and other methods now use the property_hash where appropriate. Tests rewritten to handle changes and extend code coverage.
Modify manifests and tests to handle the renamed providers.
|
OK, original types back with deprecation warnings. |
| @@ -6,6 +6,10 @@ | |||
|
|
|||
| newparam(:name, :namevar=>true) do | |||
| desc 'The name of the database.' | |||
| validate do |value| | |||
| Puppet.warning("database has been deprecated in favor of mysql_database.") | |||
There was a problem hiding this comment.
Whats the difference between Puppet.warning and Puppet.deprecation_warning ? Should you be using the latter?
There was a problem hiding this comment.
Aha, Puppet.warning doesn't attempt to give an incorrect line number :)
There was a problem hiding this comment.
I would pretend that was intentional but I hadn't come across .deprecation_warning, so I had no idea it even existed.
This provider has undergone the largest set of changes and currently just accepts a full SQL grant string as the name and then applies it, making things easier for DBAs and removes the awkward attempts at modelling grants into Puppet.
WIP: database_user and database refactoring
WIP WIP WIP DON'T MERGE:
This PR refactors the providers to rename them from database_X to mysql_X and attempts to bring in several improvements, from better instances to prefetch support.