Option for create nfs exported directory #186
Conversation
bschonec
force-pushed
the
option_for_create_nfs_exported_directory
branch
from
5eb710c
to
4916df4
Compare
|
@bschonec Can you please rebase your branch? |
bschonec
force-pushed
the
option_for_create_nfs_exported_directory
branch
from
b2e91e8
to
a164ebe
Compare
|
@Tux, sure thing. I'm not sure where I should put the tests for the directory (not) creation. There are a lot of places in the tests where it does "ensure directory." I only need to test for "not ensure directory." Should I just create a simple nfsv4/nfsv3 section and do a "not ensure directory" and not test for anything else? |
manifests/functions/create_export.pp
Outdated
| mode => $mode, | ||
| selinux_ignore_defaults => true, | ||
| if $create_dir { | ||
| filepath { $name: |
There was a problem hiding this comment.
Why the switch of file into filepath? That's not relevant when it comes to creating the folder or not and there is another PR for this change.
I would argue that this module almost never should create this folder as that normally is the responsibility of another module. I think it would be even worse to let it create an entire path ...
It would be a breaking change, but I would consider setting $create_dir default to false.
There was a problem hiding this comment.
Please stay with file for now. Besides this: there are two implementations for creating recursive directories: filepath and dirtrtee. We should have this change in a separate PR so we can check and review which module is the better one to use.
There was a problem hiding this comment.
And the module it self has this https://github.com/voxpupuli/puppet-nfs/blob/master/manifests/functions/mkdir.pp :)
bschonec
force-pushed
the
option_for_create_nfs_exported_directory
branch
3 times, most recently
from
1a9fa3c
to
79e32d1
Compare
| @@ -57,7 +57,9 @@ | |||
| # @param v4_export_name | |||
| # @param nfsv4_bindmount_enable | |||
| # | |||
| # @examples | |||
There was a problem hiding this comment.
please re-add the @examples strings tag.
There was a problem hiding this comment.
I think it should be @example, and it's is a tag used by puppet strings to render an example in REFERENCE.md
See https://www.puppet.com/docs/puppet/8/puppet_strings.html
It should be moved below the last @param at line 65
There was a problem hiding this comment.
No need to have another PR. please fix it here and please keep in mind that the code example must be indented by 2 spaces.
There was a problem hiding this comment.
I've fixed it here but I have no idea what you're referring to when you say, "...code example must be indented by 2 spaces."
There was a problem hiding this comment.
@bschonec the example should have a description, then the example code. The code should be indented two spaces, so there should be three spaces after the hash mark #. Examples: https://github.com/search?q=repo%3Avoxpupuli%2Fpuppet-systemd%20%40example&type=code
There was a problem hiding this comment.
Tag was readded. This is OK for me. We can solve this conversation.
bschonec
force-pushed
the
option_for_create_nfs_exported_directory
branch
2 times, most recently
from
b3a02f8
to
6e9b743
Compare
bschonec
force-pushed
the
option_for_create_nfs_exported_directory
branch
from
6e9b743
to
bcf5d29
Compare
I have a case where an application I'm installing expects that the folders to be exported via NFS do NOT exist at application install time.
This PR allows for the edge case where you don't want to create the directories but you do want to manage the NFS export file(s).