Re: [PR] NIFI-12500: Add dynamic target for Get/Set/SendTrapSNMP [nifi]
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]
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]
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]
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]
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]
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]
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]
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]
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