Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2024-01-25 Thread via GitHub


tpalfy commented on PR #8160:
URL: https://github.com/apache/nifi/pull/8160#issuecomment-1910301209

   Fixed some Java 8 compatibility issues and pushed to nifi-1.x


-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2024-01-25 Thread via GitHub


tpalfy commented on PR #8160:
URL: https://github.com/apache/nifi/pull/8160#issuecomment-1910260542

   LGTM
   Thank you for your work @Lehel44 .
   Merged to main, merging to 1.x.


-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2024-01-25 Thread via GitHub


asfgit closed pull request #8160: NIFI-12500: Add dynamic target for 
Get/Set/SendTrapSNMP
URL: https://github.com/apache/nifi/pull/8160


-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2024-01-24 Thread via GitHub


tpalfy commented on code in PR #8160:
URL: https://github.com/apache/nifi/pull/8160#discussion_r1465482740


##
nifi-nar-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/test/java/org/apache/nifi/snmp/operations/SNMPRequestIT.java:
##
@@ -237,9 +243,11 @@ void testSnmpGetInvalidOidWithFlowFileInput(int version, 
SNMPConfigurationFactor
 agent.start();
 try {
 final SNMPConfiguration snmpConfiguration = 
snmpConfigurationFactory.createSnmpGetSetConfiguration(agent.getPort());
-snmpResourceHandler = 
SNMPFactoryProvider.getFactory(version).createSNMPResourceHandler(snmpConfiguration);
-final GetSNMPHandler getSNMPHandler = new 
GetSNMPHandler(snmpResourceHandler);
-final Optional optionalResponse = 
getSNMPHandler.get(getFlowFileAttributesForSnmpGet(INVALID_OID, 
READ_ONLY_OID_2));
+final SNMPContext factory = 
SNMPFactoryProvider.getFactory(version);
+final Target target = 
factory.createTargetInstance(snmpConfiguration);
+final Snmp snmpManager = 
factory.createSnmpManagerInstance(snmpConfiguration);
+final GetSNMPHandler getSNMPHandler = new 
GetSNMPHandler(snmpManager);
+final Optional optionalResponse = 
getSNMPHandler.get(getFlowFileAttributesForSnmpGet(INVALID_OID, 
READ_ONLY_OID_2), target);
 if (optionalResponse.isPresent()) {
 final SNMPSingleResponse response = optionalResponse.get();
 if (version == SnmpConstants.version1) {

Review Comment:
   Unfortunately this integration test fails because the order in which the 
OIDs are added to the request and hence the order in which they are in the 
response is not deterministic.
   
   I suggest changing lines 253-261 to this:
   ```java
   final List actualVariableBindings = 
response.getVariableBindings();
   final List expectedVariableBindings;
   
   final List> equalsProperties = 
Arrays.asList(
   SNMPValue::getOid,
   SNMPValue::getVariable
   );
   
   if (version == SnmpConstants.version1) {
   expectedVariableBindings = Arrays.asList(
   new SNMPValue(INVALID_OID, "Null"),
   new SNMPValue(READ_ONLY_OID_2, 
READ_ONLY_OID_VALUE_2)
   );
   assertEquals(NO_SUCH_NAME, 
response.getErrorStatusText());
   } else {
   expectedVariableBindings = Arrays.asList(
   new SNMPValue(INVALID_OID, NO_SUCH_OBJECT),
   new SNMPValue(READ_ONLY_OID_2, 
READ_ONLY_OID_VALUE_2)
   );
   assertEquals(SUCCESS, response.getErrorStatusText());
   }
   assertEquals(
   new 
HashSet<>(EqualsWrapper.wrapList(actualVariableBindings, equalsProperties)),
   new 
HashSet<>(EqualsWrapper.wrapList(expectedVariableBindings, equalsProperties))
   );
   ```



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2024-01-24 Thread via GitHub


Lehel44 commented on code in PR #8160:
URL: https://github.com/apache/nifi/pull/8160#discussion_r1465352063


##
nifi-nar-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/main/java/org/apache/nifi/snmp/processors/SendTrapSNMP.java:
##
@@ -147,17 +158,18 @@ public void onTrigger(final ProcessContext context, final 
ProcessSession process
 attributes.put("enterpriseOid", enterpriseOid);
 attributes.put("genericTrapType", genericTrapType);
 attributes.put("specificTrapType", specificTrapType);
-snmpHandler.sendTrap(attributes, v1TrapConfiguration);
+snmpHandler.sendTrap(attributes, v1TrapConfiguration, target);
 } else {
 final String trapOidValue = 
context.getProperty(V2TrapProperties.TRAP_OID_VALUE).evaluateAttributeExpressions(flowFile).getValue();
 V2TrapConfiguration v2TrapConfiguration = new 
V2TrapConfiguration(trapOidValue);
 attributes.put("trapOidValue", trapOidValue);
-snmpHandler.sendTrap(attributes, v2TrapConfiguration);
+snmpHandler.sendTrap(attributes, v2TrapConfiguration, target);
 }
-
-processSession.putAllAttributes(flowFile, attributes);
+flowFile = processSession.putAllAttributes(flowFile, attributes);
 processSession.transfer(flowFile, REL_SUCCESS);
-
+if (isFlowFileReceived) {
+processSession.getProvenanceReporter().receive(flowFile, 
"/sendTrap");
+}

Review Comment:
   The data is not from an external system, I agree, removing.



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2024-01-24 Thread via GitHub


tpalfy commented on code in PR #8160:
URL: https://github.com/apache/nifi/pull/8160#discussion_r1465309276


##
nifi-nar-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/main/java/org/apache/nifi/snmp/processors/SendTrapSNMP.java:
##
@@ -147,17 +158,18 @@ public void onTrigger(final ProcessContext context, final 
ProcessSession process
 attributes.put("enterpriseOid", enterpriseOid);
 attributes.put("genericTrapType", genericTrapType);
 attributes.put("specificTrapType", specificTrapType);
-snmpHandler.sendTrap(attributes, v1TrapConfiguration);
+snmpHandler.sendTrap(attributes, v1TrapConfiguration, target);
 } else {
 final String trapOidValue = 
context.getProperty(V2TrapProperties.TRAP_OID_VALUE).evaluateAttributeExpressions(flowFile).getValue();
 V2TrapConfiguration v2TrapConfiguration = new 
V2TrapConfiguration(trapOidValue);
 attributes.put("trapOidValue", trapOidValue);
-snmpHandler.sendTrap(attributes, v2TrapConfiguration);
+snmpHandler.sendTrap(attributes, v2TrapConfiguration, target);
 }
-
-processSession.putAllAttributes(flowFile, attributes);
+flowFile = processSession.putAllAttributes(flowFile, attributes);
 processSession.transfer(flowFile, REL_SUCCESS);
-
+if (isFlowFileReceived) {
+processSession.getProvenanceReporter().receive(flowFile, 
"/sendTrap");
+}

Review Comment:
   I think we don't need to add a provenance event explicitly. No data (not 
even attribute) is getting value from any outside sources. Instead, an 
ATTRIBUTES_UPDATED will be generated automatically.
   
   ```suggestion
   ```



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2024-01-24 Thread via GitHub


tpalfy commented on code in PR #8160:
URL: https://github.com/apache/nifi/pull/8160#discussion_r1465266010


##
nifi-nar-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/main/java/org/apache/nifi/snmp/processors/SetSNMP.java:
##
@@ -101,28 +102,30 @@ public class SetSNMP extends AbstractSNMPProcessor {
 @OnScheduled
 public void init(final ProcessContext context) {
 initSnmpManager(context);
-snmpHandler = new SetSNMPHandler(snmpResourceHandler);
+snmpHandler = new SetSNMPHandler(snmpManager);
 }
 
 @Override
 public void onTrigger(final ProcessContext context, final ProcessSession 
processSession) {
 final FlowFile flowFile = processSession.get();
 if (flowFile != null) {
 try {
-final Optional optionalResponse = 
snmpHandler.set(flowFile.getAttributes());
+final Target target = 
factory.createTargetInstance(getTargetConfiguration(context, flowFile));
+final Optional optionalResponse = 
snmpHandler.set(flowFile.getAttributes(), target);
 if (optionalResponse.isPresent()) {
 processSession.remove(flowFile);
 final FlowFile outgoingFlowFile = processSession.create();
 final SNMPSingleResponse response = optionalResponse.get();
-
processSession.getProvenanceReporter().receive(outgoingFlowFile, "/set");
-handleResponse(context, processSession, outgoingFlowFile, 
response, REL_SUCCESS, REL_FAILURE, "/set");
+handleResponse(context, processSession, outgoingFlowFile, 
response, REL_SUCCESS, REL_FAILURE, "/set", true);
 } else {
 getLogger().warn("No SNMP specific attributes found in 
flowfile.");
 processSession.transfer(flowFile, REL_FAILURE);
+processSession.getProvenanceReporter().receive(flowFile, 
"/set");
 }
 } catch (IOException e) {
 getLogger().error("Failed to send request to the agent. Check 
if the agent supports the used version.");
 processSession.transfer(processSession.penalize(flowFile), 
REL_FAILURE);
+processSession.getProvenanceReporter().receive(flowFile, 
"/set");

Review Comment:
   ```suggestion
   ```



##
nifi-nar-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/main/java/org/apache/nifi/snmp/processors/SetSNMP.java:
##
@@ -101,28 +102,30 @@ public class SetSNMP extends AbstractSNMPProcessor {
 @OnScheduled
 public void init(final ProcessContext context) {
 initSnmpManager(context);
-snmpHandler = new SetSNMPHandler(snmpResourceHandler);
+snmpHandler = new SetSNMPHandler(snmpManager);
 }
 
 @Override
 public void onTrigger(final ProcessContext context, final ProcessSession 
processSession) {
 final FlowFile flowFile = processSession.get();
 if (flowFile != null) {
 try {
-final Optional optionalResponse = 
snmpHandler.set(flowFile.getAttributes());
+final Target target = 
factory.createTargetInstance(getTargetConfiguration(context, flowFile));
+final Optional optionalResponse = 
snmpHandler.set(flowFile.getAttributes(), target);
 if (optionalResponse.isPresent()) {
 processSession.remove(flowFile);
 final FlowFile outgoingFlowFile = processSession.create();
 final SNMPSingleResponse response = optionalResponse.get();
-
processSession.getProvenanceReporter().receive(outgoingFlowFile, "/set");
-handleResponse(context, processSession, outgoingFlowFile, 
response, REL_SUCCESS, REL_FAILURE, "/set");
+handleResponse(context, processSession, outgoingFlowFile, 
response, REL_SUCCESS, REL_FAILURE, "/set", true);
 } else {
 getLogger().warn("No SNMP specific attributes found in 
flowfile.");
 processSession.transfer(flowFile, REL_FAILURE);
+processSession.getProvenanceReporter().receive(flowFile, 
"/set");

Review Comment:
   ```suggestion
   ```



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2024-01-24 Thread via GitHub


tpalfy commented on code in PR #8160:
URL: https://github.com/apache/nifi/pull/8160#discussion_r1465260947


##
nifi-nar-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/main/java/org/apache/nifi/snmp/processors/GetSNMP.java:
##
@@ -148,97 +148,89 @@ public class GetSNMP extends AbstractSNMPProcessor {
 @OnScheduled
 public void init(final ProcessContext context) {
 initSnmpManager(context);
-snmpHandler = new GetSNMPHandler(snmpResourceHandler);
+snmpHandler = new GetSNMPHandler(snmpManager);
 }
 
 @Override
 public void onTrigger(final ProcessContext context, final ProcessSession 
processSession) {
 final SNMPStrategy snmpStrategy = 
SNMPStrategy.valueOf(context.getProperty(SNMP_STRATEGY).getValue());
 final String oid = context.getProperty(OID).getValue();
-final FlowFile flowfile = processSession.get();
+final boolean isNewFlowFileCreated;
+FlowFile flowfile = processSession.get();
+if (flowfile == null) {
+isNewFlowFileCreated = true;
+flowfile = processSession.create();
+} else {
+isNewFlowFileCreated = false;
+}
 
+final Target target = 
factory.createTargetInstance(getTargetConfiguration(context, flowfile));
 if (SNMPStrategy.GET == snmpStrategy) {
-performSnmpGet(context, processSession, oid, flowfile);
+performSnmpGet(context, processSession, oid, target, flowfile, 
isNewFlowFileCreated);
 } else if (SNMPStrategy.WALK == snmpStrategy) {
-performSnmpWalk(context, processSession, oid, flowfile);
+performSnmpWalk(context, processSession, oid, target, flowfile, 
isNewFlowFileCreated);
 }
 }
 
 void performSnmpWalk(final ProcessContext context, final ProcessSession 
processSession, final String oid,
- final FlowFile flowFile) {
+ final Target target, FlowFile flowFile, final boolean 
isNewFlowFileCreated) {
+
+if (oid != null) {
+String prefixedOid = SNMPUtils.SNMP_PROP_PREFIX + oid;
+flowFile = processSession.putAttribute(flowFile, prefixedOid, "");
+}
+
 try {
-if (flowFile != null) {
-performSnmpWalkWithFlowFile(processSession, flowFile);
+final Optional optionalResponse = 
snmpHandler.walk(flowFile.getAttributes(), target);
+if (optionalResponse.isPresent()) {
+final SNMPTreeResponse response = optionalResponse.get();
+response.logErrors(getLogger());
+flowFile = processSession.putAllAttributes(flowFile, 
response.getAttributes());
+
processSession.getProvenanceReporter().modifyAttributes(flowFile, 
response.getTargetAddress() + "/walk");
+if (isNewFlowFileCreated) {
+processSession.getProvenanceReporter().fetch(flowFile, 
"/walk");
+} else {
+processSession.getProvenanceReporter().receive(flowFile, 
"/walk");
+}

Review Comment:
   ```suggestion
   if (isNewFlowFileCreated) {
   processSession.getProvenanceReporter().receive(flowFile, 
"/walk");
   } else {
   processSession.getProvenanceReporter().fetch(flowFile, 
"/walk");
   }
   ```



##
nifi-nar-bundles/nifi-snmp-bundle/nifi-snmp-processors/src/main/java/org/apache/nifi/snmp/processors/GetSNMP.java:
##
@@ -148,97 +148,89 @@ public class GetSNMP extends AbstractSNMPProcessor {
 @OnScheduled
 public void init(final ProcessContext context) {
 initSnmpManager(context);
-snmpHandler = new GetSNMPHandler(snmpResourceHandler);
+snmpHandler = new GetSNMPHandler(snmpManager);
 }
 
 @Override
 public void onTrigger(final ProcessContext context, final ProcessSession 
processSession) {
 final SNMPStrategy snmpStrategy = 
SNMPStrategy.valueOf(context.getProperty(SNMP_STRATEGY).getValue());
 final String oid = context.getProperty(OID).getValue();
-final FlowFile flowfile = processSession.get();
+final boolean isNewFlowFileCreated;
+FlowFile flowfile = processSession.get();
+if (flowfile == null) {
+isNewFlowFileCreated = true;
+flowfile = processSession.create();
+} else {
+isNewFlowFileCreated = false;
+}
 
+final Target target = 
factory.createTargetInstance(getTargetConfiguration(context, flowfile));
 if (SNMPStrategy.GET == snmpStrategy) {
-performSnmpGet(context, processSession, oid, flowfile);
+performSnmpGet(context, processSession, oid, target, flowfile, 
isNewFlowFileCreated);
 } else if (SNMPStrategy.WALK == snmpStrategy) {
-performSnmpWalk(context, processSession, oid, flowfile);
+performS

[PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]

2023-12-14 Thread via GitHub


Lehel44 opened a new pull request, #8160:
URL: https://github.com/apache/nifi/pull/8160

   
   
   
   
   
   
   
   
   
   
   
   
   
   # Summary
   
   [NIFI-12500](https://issues.apache.org/jira/browse/NIFI-12500)
   
   # Tracking
   
   Please complete the following tracking steps prior to pull request creation.
   
   ### Issue Tracking
   
   - [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue 
created
   
   ### Pull Request Tracking
   
   - [ ] Pull Request title starts with Apache NiFi Jira issue number, such as 
`NIFI-0`
   - [ ] Pull Request commit message starts with Apache NiFi Jira issue number, 
as such `NIFI-0`
   
   ### Pull Request Formatting
   
   - [ ] Pull Request based on current revision of the `main` branch
   - [ ] Pull Request refers to a feature branch with one commit containing 
changes
   
   # Verification
   
   Please indicate the verification steps performed prior to pull request 
creation.
   
   ### Build
   
   - [ ] Build completed using `mvn clean install -P contrib-check`
 - [ ] JDK 21
   
   ### Licensing
   
   - [ ] New dependencies are compatible with the [Apache License 
2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License 
Policy](https://www.apache.org/legal/resolved.html)
   - [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` 
files
   
   ### Documentation
   
   - [ ] Documentation formatting appears as expected in rendered files
   


-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org