Refactor volume options fact as array #197
Conversation
|
Anything needed for this to be merged? |
|
Dear @svenbs, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
Dear @svenbs, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
svenbs
force-pushed
the
master
branch
5 times, most recently
from
ec13d97
to
f164be1
Compare
|
Dear @svenbs, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs? |
Volume options are stored as comma separated string, the only use from the gluster module is in volume.pp, there it is converted back to an array using split(). Some gluster volume options are using commas for splitting values (e.g. auth.allow), this conflicts with this behaviour.
|
Dear @svenbs, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
Ping. Any news on this? |
Pull Request (PR) description
Volume options are stored as comma separated string, the only use from
the gluster module is in volume.pp, there it is converted back to an
array using split(). Some gluster volume options are using commas for
splitting values (e.g. auth.allow), this conflicts with this behaviour.
This change is BREAKING as it will change the output of the gluster_volume_#{vol}_options fact from a comma separated string to an actual array data type.
As the fact could be used by user implementations it should be marked in the release notes as ACTION REQUIRED.
This Pull Request (PR) fixes the following issues
Fixes #53
Fixes #165
The above issues are addressing this problem and should be solved by #186. We were waiting for #186 to make progress and therefore created this minor change for ourselves.
It seems #186 is a bigger effort to achieve and also has a big impact as it will introduce major changes to the API of the module. Maybe it would be helpful to fix #53 with minor impact and then go on with the bigger effort in #186.