szaszm commented on code in PR #2016:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2016#discussion_r2308193434
##########
extensions/azure/utils/AzureEnums.h:
##########
@@ -19,11 +19,39 @@
*/
#pragma once
+#include "magic_enum.hpp"
+
namespace org::apache::nifi::minifi::azure {
enum class EntityTracking {
none,
timestamps
};
+enum class CredentialConfigurationStrategyOption {
+ fromProperties,
+ defaultCredential,
+ managedIdentity,
+ workloadIdentity
+};
Review Comment:
I'd prefer to keep the enum members as UpperCamelCase or UPPER_SNAKE_CASE
##########
extensions/azure/controllerservices/AzureStorageCredentialsService.h:
##########
@@ -42,33 +43,44 @@ class AzureStorageCredentialsService : public
core::controller::ControllerServic
.build();
EXTENSIONAPI static constexpr auto StorageAccountKey =
core::PropertyDefinitionBuilder<>::createProperty("Storage Account Key")
.withDescription("The storage account key. This is an admin-like
password providing access to every container in this account. "
- "It is recommended one uses Shared Access Signature (SAS) token
instead for fine-grained control with policies.")
+ "It is recommended one uses Shared Access Signature
(SAS) token instead for fine-grained control with policies "
+ "if Credential Configuration Strategy is set to From
Properties. If set, SAS Token must be empty.")
Review Comment:
I'd prefer to keep continuation indentations = two normal levels of
indentation, and not aligned with whatever from the previous lines.
Why?
- saves horizontal space
- prevents refactor/rename from messing up the formatting
- still lets us differentiate between normal indentation and continuation of
the previous thing
##########
extensions/azure/controllerservices/AzureStorageCredentialsService.cpp:
##########
@@ -29,6 +29,10 @@ void AzureStorageCredentialsService::initialize() {
}
void AzureStorageCredentialsService::onEnable() {
+ if (auto credential_configuration_strategy =
magic_enum::enum_cast<CredentialConfigurationStrategyOption>(
+
getProperty(CredentialConfigurationStrategy.name).value_or(std::string{magic_enum::enum_name(CredentialConfigurationStrategyOption::fromProperties)})))
{
Review Comment:
Instead of breaking the line in two, I'd extract parts of it to keep it
simpler
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
