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.

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

Support multiple lines of the same option #398

Merged
merged 1 commit into from
Dec 18, 2013

Conversation

fridim
Copy link
Contributor

@fridim fridim commented Dec 16, 2013

Some MySQL options need to be passed several time. Example :

http://dev.mysql.com/doc/refman/5.5/en/replication-options-slave.html#option_mysqld_replicate-do-db

This is currently impossible with the override_options. This patch allows to
pass array as value. Example :

override_options => {
  'client' => {
    'password' => 'xxx',
  },
  'mysqld' => {
    'bind_address'    => '0.0.0.0',
    'replicate-do-db' => ['base1', 'base2', 'base3'],
  },
}

Which will be evaluated into :

[client]
password = xxx

[mysqld]
bind_address = 0.0.0.0
replicate-do-db = base1
replicate-do-db = base2
replicate-do-db = base3

@igalic
Copy link
Contributor

igalic commented Dec 16, 2013

Do we have a safe list of options which can occur multiple times?

@fridim
Copy link
Contributor Author

fridim commented Dec 16, 2013

Not sure if such a list exists. I asked #mariadb on IRC, they don't think so.

At least 9 of replication-related options can have multiple instances:

  • replicate-wild-ignore-table
  • replicate-do-db
  • replicate-wild-do-table
  • replicate-ignore-db
  • replicate-do-table
  • replicate-ignore-table
  • replicate-rewrite-db
  • binlog-do-db
  • binlog-ignore-db

@igalic
Copy link
Contributor

igalic commented Dec 16, 2013

can you please add a test and documentation?

@fridim
Copy link
Contributor Author

fridim commented Dec 16, 2013

sure

@fridim fridim closed this Dec 16, 2013
@fridim fridim reopened this Dec 16, 2013
@fridim
Copy link
Contributor Author

fridim commented Dec 17, 2013

Commit updated with doc and test.

Is the test in the right place ? Let me know (was not sure about it).

@igalic
Copy link
Contributor

igalic commented Dec 17, 2013

I'm mildly confused that we still have spec/system, since after @apenney switched us over to beaker

Generally there are two kinds of tests that we have "system" tests (which are now relocated in spec/acceptance) and "normal" rspec puppet tests (everything else in spec/). Please look at our sister-module's contribution guidelines for more info.

@fridim
Copy link
Contributor Author

fridim commented Dec 18, 2013

Tests added. As suggested on IRC, added test for

  • spec/classes/mysql_server_spec.rb
  • both spec/system/mysql_server_spec.rb and spec/acceptance/mysql_server_spec.rb

@apenney
Copy link
Contributor

apenney commented Dec 18, 2013

Thanks for the additional tests! I should probably just yank out spec/system/ now, I just wanted to be sure we were done with the beaker stuff before doing so. I'll take a look at this, mysql makes me sad with how complex my.cnf can be. :)

  Some MySQL options need to be passed several times. For example :

  http://dev.mysql.com/doc/refman/5.5/en/replication-options-slave.html#option_mysqld_replicate-do-db

  This is currently impossible with the override_options. This patch allows to
  pass array as value. Example :

    override_options => {
      'client' => {
        'password' => 'xxx',
      },
      'mysqld' => {
        'bind_address'    => '0.0.0.0',
        'replicate-do-db' => ['base1', 'base2', 'base3'],
      },
    }

  Which will be evaluated into :

    [client]
    password = xxx

    [mysqld]
    bind_address = 0.0.0.0
    replicate-do-db = base1
    replicate-do-db = base2
    replicate-do-db = base3
@fridim
Copy link
Contributor Author

fridim commented Dec 18, 2013

Just rebased from puppetlabs-mysql/master

apenney pushed a commit that referenced this pull request Dec 18, 2013
Support multiple lines of the same option
@apenney apenney merged commit 3736fcb into puppetlabs:master Dec 18, 2013
@apenney
Copy link
Contributor

apenney commented Dec 18, 2013

Thanks for this, I had no idea this was allowed in my.cnf, very cool. :)

@fridim fridim deleted the support_multiple_values_in_config branch December 19, 2013 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants