NIFI-7307 Adding support for setting and retrieving Azure Blob Storag…#10939
Conversation
pvillard31
left a comment
pvillard31
left a comment
There was a problem hiding this comment.
Left some initial comments. Will be also important to add some test coverage.
|
Thank you for your review and feedback, @pvillard31. I have made all of the changes you requested and will look into adding tests today. It looks like all of the current Azure blob storage tests only focus on property migration, so it might take me a little time to add for these processors. |
0dc3382 to
7436977
Compare
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for working on this @pkelly-nifi.
Although other Azure Processors have the _v12 suffix, I don't think that convention should be perpetuated for new Processors, so I recommend renaming these new Processors.
|
Also important to note, all commits must have verified signatures, so it will be necessary to rebase and force-push to the branch to ensure all commits are signed. |
|
Thank you for your feedback @exceptionfactory. I renamed the new processors to remove "_v12" from their names and rebased with signing the previous commit. |
|
Hi @pvillard31 and @exceptionfactory. Do you have any additional feedback for these processors? |
pvillard31
left a comment
pvillard31
left a comment
There was a problem hiding this comment.
Please rebase on top of origin/main for the checks to properly execute.
pvillard31
left a comment
pvillard31
left a comment
There was a problem hiding this comment.
Latest LGTM but I just thought that maybe FetchAzure... would be better than GetAzure... as most of the time Get processors are processors with no input relationship. Thoughts?
| public static final PropertyDescriptor BLOB_TAG_PREFIX = new PropertyDescriptor.Builder() | ||
| .name("Blob Tag Prefix") | ||
| .description("Specifies the prefix which would be scanned against the incoming FlowFile's attributes and the matching attribute's " + | ||
| "name and value would be considered as the outgoing Azure blob's Tag name and Tag value respectively. For Ex: If the " + | ||
| "incoming FlowFile carries the attributes tagAzurecountry, tagAzurePII, the tag prefix to be specified would be 'tagAzure'") | ||
| .required(false) | ||
| .addValidator(StandardValidators.NON_EMPTY_EL_VALIDATOR) | ||
| .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) | ||
| .build(); | ||
|
|
||
| public static final PropertyDescriptor REMOVE_TAG_PREFIX = new PropertyDescriptor.Builder() | ||
| .name("Remove Tag Prefix") | ||
| .description("If set to 'True', the value provided for '" + BLOB_TAG_PREFIX.getDisplayName() + "' will be removed from " + | ||
| "the attribute(s) and then considered as the Tag name. For ex: If the incoming FlowFile carries the attributes tagAzurecountry, " + | ||
| "tagAzurePII and the prefix is set to 'tagAzure' then the corresponding tag values would be 'country' and 'PII'") | ||
| .allowableValues(new AllowableValue("true", "True"), new AllowableValue("false", "False")) | ||
| .defaultValue("false") | ||
| .build(); |
There was a problem hiding this comment.
Can we use multi line strings here?
|
I went back and forth on the Get/Fetch name at first, but went with Get to keep it consistent with the equivalent S3 processors. I agree that Fetch is more appropriate for this. Will update. |
…e metadata and tags
|
Looks like the checks were failing. Rebased again. |
3 participants
…e metadata and tags
Summary
NIFI-7307 Adds support for setting and retrieving Azure Blob Storage object metadata and tags. Modeled after similar functionality which exists for S3.
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation