(MODULES-552) Add capability to specify column_privileges #570
Conversation
|
could you please squash those two commits? |
|
O.k.. |
|
sorry! i usually include a link to the documentation for rebase and squash, just in case someone is new. |
fnerdwq
force-pushed
the
mysql_grant_column_privs
branch
from
859b55d to
a4b84a9
Compare
|
Thanks, "so far so clear" ;-) So that's what I did now... Hope this if fine... |
|
it's different from project to project. we're fine with it. |
| stripped_privileges = privileges.strip.split(/\s*,\s*(?![^(]*\))/).map{ |priv| | ||
| # split and sort the column_privileges in the parentheses and rejoin | ||
| if priv.include?('(') | ||
| type, col=priv.match(/\s*(\S*)\s*\(\s*(.*)\s*\)/).captures |
There was a problem hiding this comment.
why isn't this simply a strip.split('\s+', 2) ?
There was a problem hiding this comment.
Ah, I think you are right. I was so focused on regexes yesterday (it made my head some to get the one in line 35 done ;-)
I'll change that.
There was a problem hiding this comment.
Now I got it... Since it's not quaranteed, that there is a space between the keyword and the bracked (at least not in the type) it should be: strip.split(/\s+|\b/,2). Thanks for that simplification.
But then the next line gets more complicated
type.upcase + " (" + col.slice(1...-1).strip.split(/\s*,\s*/).sort.join(', ') + ")"
due to the parentheses.
fnerdwq
force-pushed
the
mysql_grant_column_privs
branch
from
a4b84a9 to
0407a99
Compare
|
Updated for your request to avoid the regex. |
| # split on ',' if it is not a non-'('-containing string followed by a | ||
| # closing parenthesis ')'-char - e.g. only split comma separated elements not in | ||
| # parentheses | ||
| stripped_privileges = privileges.strip.split(/\s*,\s*(?![^(]*\))/).map{ |priv| |
There was a problem hiding this comment.
Multi-line blocks should use do/end over {/}
There was a problem hiding this comment.
Ok, corrected.
Is this also 'best-practice' if there are more method call chained on the block, like do ... end.sort?
fnerdwq
force-pushed
the
mysql_grant_column_privs
branch
from
0407a99 to
cdd8b7b
Compare
|
@cmurphy halp plz. my eyes hurt |
| it 'ordered including column privileges' do | ||
| @user = Puppet::Type.type(:mysql_grant).new( | ||
| :name => 'foo@localhost/*.*', :table => ['*.*','@'], :user => 'foo@localhost', | ||
| :privileges => ['SELECT(Host,Address)', 'Proxy'] ) |
There was a problem hiding this comment.
The PROXY privilege must be named by itself, so while this test passes, applying it in a manifest doesn't work:
Error: Execution of '/usr/bin/mysql -e GRANT PROXY, SELECT (a, b) ON *.* TO 'nss-user'@'%' WITH GRANT OPTION' returned 1: ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'PROXY, SELECT (a, b) ON *.* TO 'nss-user'@'%' WITH GRANT OPTION' at line 1
Could you change the test to have privileges ['SELECT(Host,Address)', 'INSERT'] or something similar just so it's not misleading?
|
Looks good to me except for the spec issue. |
fnerdwq
force-pushed
the
mysql_grant_column_privs
branch
from
cdd8b7b to
80a0e10
Compare
|
I changed the spec file as you proposed... |
|
super! thanks! |
fnerdwq
force-pushed
the
mysql_grant_column_privs
branch
from
80a0e10 to
d5e319a
Compare
fnerdwq
force-pushed
the
mysql_grant_column_privs
branch
from
d5e319a to
f88719b
Compare
|
I added an extra example under mysql_grant. Hope that's enough? |
(MODULES-552) Add capability to specify column_privileges
|
absolutely, thanks! |
Fixes Jira Issue MODULES-552.