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.

Sign up for GitHub

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

Jump to bottom

Repication custom resource #151

Merged
merged 4 commits into from
Mar 30, 2017
Merged

Repication custom resource #151

merged 4 commits into from
Mar 30, 2017

Conversation

dimsh99
Copy link
Contributor

@dimsh99 dimsh99 commented Mar 24, 2017

This PR converts mariadb_replication LWRP into custom resource, introduces mysql2_gem resource ( mysql2_chef_gem cookbook caused circular dependency error ). RH installation recipes for server were united. Some time was spent on centos_spec.

@dimsh99
Logic of MariaDB package & service names determination moved to helper
fa9ebe1 
Split devel from client to install only devel packages when needed.
@dimsh99
Changed LWRP mariadb_replication to custom resource.
d1c7eb7 
Added custom_resource mysql2_gem, since cookbook mysql2_chef_gem
caused circular dependency error.
Fixed cookbook for replication testing.
@dimsh99
Changes in specs
54ec17f
@dimsh99
Updated documentation
94e0fe7
@shoekstra
Copy link
Contributor

Looks good! There is a quite a bit to digest here and will need to take a proper look before merging.

@shoekstra
Copy link
Contributor

shoekstra commented Mar 29, 2017 •
edited
Loading

First off, it would be a lot easier if these were separate PRs, so that each change could be merged in and/or discussed individually, such as:

  • Converting mariadb_replication to a custom resource (including Adding and implementing MariaDB::Connection::Helper)
  • Refactoring and moving logic into MariaDB::Helper
  • Adding the devel recipe
  • Cleaning up _redhat_server_native.rb and _redhat_server_scl.rb

That said, let the discussion begin :)

shoekstra
shoekstra reviewed Mar 29, 2017
View reviewed changes
recipes/_mariadb_repository.rb
node['platform'], node['platform_version'])
# MariaDB repository
include_recipe "#{cookbook_name}::repository"
end

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

shoekstra
shoekstra reviewed Mar 29, 2017
View reviewed changes
resources/mysql2_gem.rb
Array(action).each do |action|
run_action(action)
end
end

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

@shoekstra shoekstra merged commit 97e1357 into sous-chefs:master Mar 30, 2017
@shoekstra
Copy link
Contributor

Next time please break it up a bit, will make it quicker/easier to merge in.

Thanks for the contribution! 👍

@shoekstra shoekstra mentioned this pull request Mar 30, 2017
Closed
jfwm2 pushed a commit to jfwm2/mariadb that referenced this pull request Apr 4, 2017
@JonathanTron
Change default['grafana']['package']['apt_rebuild'] to true (fixes so…
599c928 
…us-chefs#151)

This is the default for chef's `apt_repository` resource.
@jfwm2 jfwm2 mentioned this pull request Apr 8, 2017
Merged
@lock
Copy link

lock bot commented Jul 24, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shoekstra shoekstra shoekstra left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dimsh99 @shoekstra