Skip to content

(MODULES-1338) Allow mysql::db to import several files #574

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

Merged
merged 1 commit into from
Nov 14, 2014

Conversation

Spredzy
Copy link
Contributor

@Spredzy Spredzy commented Sep 20, 2014

A user might need to import several files on database creation.
Currently the module only allows the import of a single file.
This commit allows one to, from now on, import severals.

Before :

  mysql::db { 'test' :
    sql => '/tmp/my_import1.sql',
  }

Now :

  mysql::db { 'test' :
    sql => [
      '/tmp/my_import1.sql',
      '/tmp/my_import2.sql',
    ]
  }

@@ -48,7 +49,7 @@

if $sql {
exec{ "${dbname}-import":
command => "/usr/bin/mysql ${dbname} < ${sql}",
command => "/bin/ls ${sql_inputs} | /usr/bin/xargs -I {} /bin/sh -c '/usr/bin/mysql ${dbname} < {} '",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for one, i'm only now notiicing /usr/bin/mysql - that's not really cool for anything that's not linux.
i'm not happy with the use of ls & xargs here.

is there a less convoluted way to do that?

command => "cat ${sql_inputs} | mysql ${dbname}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igalic I though I tried the cat solution and it didn't work. I tried back now and it seems to work, so yes I agree to go with that one

[root@monitor sql]# cat query1.sql 
CREATE DATABASE test_1;
USE test_1;
SELECT User FROM mysql.user;
[root@monitor sql]# cat query2.sql 
CREATE DATABASE test_2;
USE test_2;
SELECT User FROM mysql.user;
[root@monitor sql]# cat query1.sql query2.sql | mysql -uroot -ppassword
User
root
root
User
root
root
[root@monitor sql]# 

For the path to mysql how would you like to see it dealt with ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only problem with cat is, if a file has no trailing \n.

generally, i'd just leave it as mysql. but i think that's for another time/pr to discuss.

at this stage we need to think how to add spec tests to cover this!

@Spredzy Spredzy force-pushed the allow_multiple_import_at_a_time branch from 13efef0 to 1321e92 Compare September 29, 2014 16:14
@Spredzy
Copy link
Contributor Author

Spredzy commented Sep 29, 2014

@igalic updated

logoutput => true,
environment => "HOME=${::root_home}",
refreshonly => $refresh,
path => '/bin:/sbin:/usr/bin:/usr/sbin',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd add '/usr/local/bin' and then we'd cover *BSD as well ;)

@igalic
Copy link
Contributor

igalic commented Sep 29, 2014

for spec tests, please take a look at our contributing doc

@Spredzy Spredzy force-pushed the allow_multiple_import_at_a_time branch from 1321e92 to 966c9c2 Compare September 29, 2014 16:42
@Spredzy
Copy link
Contributor Author

Spredzy commented Sep 29, 2014

@igalic is this test enough, or something else should be covered ?

@igalic
Copy link
Contributor

igalic commented Sep 29, 2014

let's see when travis says when it's finished.

A user might need to import several files on database creation.
Currently the module only allows the import of a single file.
This commit allows one to, from now on, import severals.

Before :

  mysql::db { 'test' :
    sql => '/tmp/my_import1.sql',
  }

Now :

  mysql::db { 'test' :
    sql => [
      '/tmp/my_import1.sql',
      '/tmp/my_import2.sql',
    ]
  }
@Spredzy Spredzy force-pushed the allow_multiple_import_at_a_time branch from 966c9c2 to 5e6a1c4 Compare November 13, 2014 19:18
@Spredzy
Copy link
Contributor Author

Spredzy commented Nov 13, 2014

up ?

@igalic
Copy link
Contributor

igalic commented Nov 14, 2014

whoops. sorry ._.

igalic added a commit that referenced this pull request Nov 14, 2014
(MODULES-1338) Allow mysql::db to import several files
@igalic igalic merged commit ab84a67 into puppetlabs:master Nov 14, 2014
@igalic
Copy link
Contributor

igalic commented Nov 14, 2014

and thanks, Alot!
ALOT

underscorgan pushed a commit to underscorgan/puppetlabs-mysql that referenced this pull request Nov 14, 2014
Remove the dependency on stdlib 4.x puppetlabs#574
introduced, add some input validation, and improve test checks.
@underscorgan underscorgan mentioned this pull request Nov 14, 2014
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.

3 participants