MINIFICPP-1410 Add permissions property support for Putfile processor #942
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With the new default directory creating permissions, the change of test directory permissions is not needed anymore.
There is an incompatilibity issue with libboost and gcc 4.8 which causes throw of std::bad_allow or segmentation fault in tests.
arpadboda
approved these changes
Nov 30, 2020
|
|
||
| #ifndef WIN32 | ||
| class FilePermissions { | ||
| static const uint32_t MINIMUM_INVALID_PERMISSIONS_VALUE = 1 << 9; |
fgerlits
reviewed
Dec 1, 2020
PROCESSORS.md
Outdated
| @@ -899,6 +899,8 @@ In the list below, the names of required properties appear in bold. Any other pr | |||
| |**Create Missing Directories**|true||If true, then missing destination directories will be created. If false, flowfiles are penalized and sent to failure.| | |||
| |Directory|.||The output directory to which to put files<br/>**Supports Expression Language: true**| | |||
| |Maximum File Count|-1||Specifies the maximum number of files that can exist in the output directory| | |||
| |Permissions|||Sets the permissions on the output file to the value of this attribute. Format must be in octal number (e.g. 644 or 0755). Not supported on Windows systems.| | |||
| |Directory Permissions|||Sets the permissions on the directories being created if 'Create Missing Directories' property is set. Format must be format octal number (e.g. 644 or 0755). Not supported on Windows systems.| | |||
There was a problem hiding this comment.
Typo: two "format"s. I would get rid of all "format"s and write "Must be an octal number (e.g. 644 or 0755)." in both property descriptions.
| core::Property PutFile::Permissions( | ||
| core::PropertyBuilder::createProperty("Permissions") | ||
| ->withDescription("Sets the permissions on the output file to the value of this attribute. " | ||
| "Format must be format octal number (e.g. 644 or 0755). Not supported on Windows systems.") |
There was a problem hiding this comment.
even more "format"s here :)
same suggestion as above
| core::PropertyBuilder::createProperty("Directory Permissions") | ||
| ->withDescription("Sets the permissions on the directories being created if 'Create Missing Directories' property is set. " | ||
| "Format must be format octal number (e.g. 644 or 0755). Not supported on Windows systems.") | ||
| ->build()); |
There was a problem hiding this comment.
What happens if these are not set? I would have expected a default value here.
There was a problem hiding this comment.
In that case it works the same way as boost::filesystem::create_directory which is creating the directory in mode 777 then applies the system's default umask.
| return S_ISDIR(dir_stat.st_mode); | ||
| } | ||
|
|
||
| inline int exists(const std::string& path) { |
There was a problem hiding this comment.
I can see that returning 0 (true) and -1 (false) fits into the existing pattern, but I think it would be better to return a bool at least from is_directory() and exists().
fgerlits
approved these changes
Dec 1, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are multiple changes done in this PR:
EXCLUDE_BOOSTis not defined and libboost is present on the system, it will be used marked with theUSE_BOOSTdefinition.PermissionsandDirectory Permissionsproperties added toPutfileprocessor for non-Windows systems for changing the permissions of the newly created output directories and the output file.777 & umaskinstead of the previous700to be in sync with the boost implementation ofboost::filesystem::create_directory.Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.