You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "Lehel44 (via GitHub)" <gi...@apache.org> on 2023/05/15 13:53:09 UTC
[GitHub] [nifi] Lehel44 opened a new pull request, #7246: NIFI-11535: Transfer ConnectWebsocket HTTP header flowfile to relatio…
Lehel44 opened a new pull request, #7246:
URL: https://github.com/apache/nifi/pull/7246
…nships
<!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
<!-- contributor license agreements. See the NOTICE file distributed with -->
<!-- this work for additional information regarding copyright ownership. -->
<!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
<!-- (the "License"); you may not use this file except in compliance with -->
<!-- the License. You may obtain a copy of the License at -->
<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
<!-- Unless required by applicable law or agreed to in writing, software -->
<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
<!-- See the License for the specific language governing permissions and -->
<!-- limitations under the License. -->
# Summary
[NIFI-11535](https://issues.apache.org/jira/browse/NIFI-11535)
# 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-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### 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 11
- [ ] JDK 17
### 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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] asfgit closed pull request #7246: NIFI-11535: Transfer ConnectWebsocket HTTP header flowfile to relatio…
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #7246: NIFI-11535: Transfer ConnectWebsocket HTTP header flowfile to relatio…
URL: https://github.com/apache/nifi/pull/7246
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] turcsanyip commented on a diff in pull request #7246: NIFI-11535: Transfer ConnectWebsocket HTTP header flowfile to relatio…
Posted by "turcsanyip (via GitHub)" <gi...@apache.org>.
turcsanyip commented on code in PR #7246:
URL: https://github.com/apache/nifi/pull/7246#discussion_r1194007689
##########
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-processors/src/main/java/org/apache/nifi/processors/websocket/AbstractWebSocketGatewayProcessor.java:
##########
@@ -77,6 +77,18 @@ public abstract class AbstractWebSocketGatewayProcessor extends AbstractSessionF
.description("The WebSocket binary message output")
.build();
+ public static final Relationship REL_SUCCESS = new Relationship.Builder()
+ .name("success")
+ .description("Connection HTTP header attributes in case of successful connection")
+ .autoTerminateDefault(true)
+ .build();
+
+ public static final Relationship REL_FAILURE = new Relationship.Builder()
+ .name("failure")
+ .description("Connection HTTP header attributes in case of connection failure")
+ .autoTerminateDefault(true)
+ .build();
+
Review Comment:
"Connection HTTP header attributes" is a bit unclear in this context and other config parameters can be passed also. I would suggest the following descriptions:
```suggestion
public static final Relationship REL_SUCCESS = new Relationship.Builder()
.name("success")
.description("FlowFile holding connection configuration attributes (like URL or HTTP headers) in case of successful connection")
.autoTerminateDefault(true)
.build();
public static final Relationship REL_FAILURE = new Relationship.Builder()
.name("failure")
.description("FlowFile holding connection configuration attributes (like URL or HTTP headers) in case of connection failure")
.autoTerminateDefault(true)
.build();
```
##########
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-processors/src/test/java/org/apache/nifi/processors/websocket/TestConnectWebSocket.java:
##########
@@ -162,10 +154,44 @@ void testDynamicUrlsParsedFromFlowFileAndAbleToConnect() throws InitializationEx
final List<MockFlowFile> flowFilesForRelationship = runner.getFlowFilesForRelationship(ConnectWebSocket.REL_CONNECTED);
assertEquals(1, flowFilesForRelationship.size());
+ final List<MockFlowFile> flowFilesForSuccess = runner.getFlowFilesForRelationship(ConnectWebSocket.REL_SUCCESS);
+ assertEquals(1, flowFilesForSuccess.size());
+
runner.stop();
webSocketListener.stop();
}
+ @Test
+ void testDynamicUrlsTransferredToFailure() throws InitializationException {
Review Comment:
The following test name would describe the test scenario more straightforward:
```suggestion
void testDynamicUrlsParsedFromFlowFileButNotAbleToConnect() throws InitializationException {
```
##########
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-processors/src/test/java/org/apache/nifi/processors/websocket/TestConnectWebSocket.java:
##########
@@ -162,10 +154,44 @@ void testDynamicUrlsParsedFromFlowFileAndAbleToConnect() throws InitializationEx
final List<MockFlowFile> flowFilesForRelationship = runner.getFlowFilesForRelationship(ConnectWebSocket.REL_CONNECTED);
assertEquals(1, flowFilesForRelationship.size());
+ final List<MockFlowFile> flowFilesForSuccess = runner.getFlowFilesForRelationship(ConnectWebSocket.REL_SUCCESS);
+ assertEquals(1, flowFilesForSuccess.size());
+
runner.stop();
webSocketListener.stop();
}
+ @Test
+ void testDynamicUrlsTransferredToFailure() throws InitializationException {
+ final TestRunner runner = TestRunners.newTestRunner(ConnectWebSocket.class);
+
+ final String serviceId = "ws-service";
+ final String endpointId = "client-1";
+
+ MockFlowFile flowFile = getFlowFile();
+ runner.enqueue(flowFile);
+
+ JettyWebSocketClient service = new JettyWebSocketClient();
+
+
+ runner.addControllerService(serviceId, service);
+ runner.setProperty(service, JettyWebSocketClient.WS_URI, "ws://localhost/12345");
Review Comment:
No need to set a fake URL because the failure comes due to ListenWebSocket not started.
```suggestion
runner.setProperty(service, JettyWebSocketClient.WS_URI, "ws://localhost/${dynamicUrlPart}");
```
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] Lehel44 commented on pull request #7246: NIFI-11535: Transfer ConnectWebsocket HTTP header flowfile to relatio…
Posted by "Lehel44 (via GitHub)" <gi...@apache.org>.
Lehel44 commented on PR #7246:
URL: https://github.com/apache/nifi/pull/7246#issuecomment-1549364306
Thanks for the review @turcsanyip!
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org