-
Notifications
You must be signed in to change notification settings - Fork 2.9k
NIFI-15156 Add support for discovering Listen Ports #10476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NIFI-15156 Add support for discovering Listen Ports #10476
Conversation
exceptionfactory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for drafting these changes @kevdoran.
It looks like there are a number of dependency version regressions in the parent pom.xml that need to be reverted.
To keep of the scope of changes more manageable, it might be helpful to decouple the ListenSyslog property changes into a separate pull request. That would also serve to highlight those changes as different than just adding the new mapping.
I plan to review more closely after reviewing the API changes, but thought it would be helpful to note these items for now.
f5a7297 to
c64ff52
Compare
|
I simplified this branch to have the bare minimum for the following:
I restructured the commits so that they are individually reviewable for each of the areas above. To test the functionality:
# nifi-extension-bundles/nifi-standard-bundle/nifi-standard-nar/target/META-INF/docs/extension-manifest.xml
<property>
<name>Listening Port</name>
<displayName>Listening Port</displayName>
<description>The Port to listen on for incoming connections</description>
<required>true</required>
<sensitive>false</sensitive>
<expressionLanguageSupported>true</expressionLanguageSupported>
<expressionLanguageScope>ENVIRONMENT</expressionLanguageScope>
<dynamicallyModifiesClasspath>false</dynamicallyModifiesClasspath>
<dynamic>false</dynamic>
<listenPortDefinition>
<transportProtocol>TCP</transportProtocol>
<applicationProtocols>
<applicationProtocol>http/1.1</applicationProtocol>
<applicationProtocol>h2</applicationProtocol>
</applicationProtocols>
</listenPortDefinition>
</property>
<property>
<name>health-check-port</name>
<displayName>Listening Port for Health Check Requests</displayName>
<required>false</required>
<sensitive>false</sensitive>
<expressionLanguageSupported>true</expressionLanguageSupported>
<expressionLanguageScope>ENVIRONMENT</expressionLanguageScope>
<dynamicallyModifiesClasspath>false</dynamicallyModifiesClasspath>
<dynamic>false</dynamic>
<listenPortDefinition>
<transportProtocol>TCP</transportProtocol>
<applicationProtocols>
<applicationProtocol>http/1.1</applicationProtocol>
<applicationProtocol>h2</applicationProtocol>
</applicationProtocols>
</listenPortDefinition>
</property>
{
"listenPorts": [
{
"portName": "Listening Port",
"portNumber": 1111,
"transportProtocol": "TCP",
"applicationProtocols": [
"http/1.1"
],
"componentType": "Processor",
"componentId": "4f328cb9-019a-1000-8f20-3fdb4fb59c8c",
"componentName": "ListenHTTP",
"componentClass": "org.apache.nifi.processors.standard.ListenHTTP",
"parentGroupId": "4f31a853-019a-1000-25c7-55eeb0410bae",
"parentGroupName": "NiFi Flow"
},
{
"portName": "Listening Port for Health Check Requests",
"portNumber": 2222,
"transportProtocol": "TCP",
"applicationProtocols": [
"http/1.1"
],
"componentType": "Processor",
"componentId": "4f328cb9-019a-1000-8f20-3fdb4fb59c8c",
"componentName": "ListenHTTP",
"componentClass": "org.apache.nifi.processors.standard.ListenHTTP",
"parentGroupId": "4f31a853-019a-1000-25c7-55eeb0410bae",
"parentGroupName": "NiFi Flow"
}
]
}
"health-check-port": {
"name": "health-check-port",
"displayName": "Listening Port for Health Check Requests",
"identifiesControllerService": false,
"sensitive": false,
"dynamic": false,
"listenPortDefinition":
{
"transportProtocol": "TCP",
"applicationProtocols":
[
"http/1.1",
"h2"
]
}
},
"Listening Port": {
"name": "Listening Port",
"displayName": "Listening Port",
"identifiesControllerService": false,
"sensitive": false,
"dynamic": false,
"listenPortDefinition":
{
"transportProtocol": "TCP",
"applicationProtocols":
[
"http/1.1",
"h2"
]
}
} |
944cb60 to
a0679e2
Compare
cccd5dc to
3d93674
Compare
3d93674 to
28bdb9d
Compare
exceptionfactory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work on the implementation @kevdoran. I noted a few very minor details, but otherwise this should be ready to go after a few adjustments.
...e/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenHTTP.java
Show resolved
Hide resolved
.../nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/dto/ListenPortDTO.java
Outdated
Show resolved
Hide resolved
...-framework-components/src/main/java/org/apache/nifi/controller/flow/AbstractFlowManager.java
Outdated
Show resolved
Hide resolved
...-framework-components/src/main/java/org/apache/nifi/controller/flow/AbstractFlowManager.java
Outdated
Show resolved
Hide resolved
...ork/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/controller/ControllerFacade.java
Outdated
Show resolved
Hide resolved
...ork/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/controller/ControllerFacade.java
Outdated
Show resolved
Hide resolved
...ork/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/controller/ControllerFacade.java
Outdated
Show resolved
Hide resolved
...ifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java
Outdated
Show resolved
Hide resolved
...on-manifest-model/src/main/java/org/apache/nifi/extension/manifest/ListenPortDefinition.java
Show resolved
Hide resolved
...on-manifest-model/src/main/java/org/apache/nifi/extension/manifest/ListenPortDefinition.java
Outdated
Show resolved
Hide resolved
|
Thanks very much for the review @exceptionfactory! I agreed with your suggestions, and have pushed a commit that should address everything. |
exceptionfactory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the work on this @kevdoran, looks good! +1 merging
Summary
NIFI-15156
Depends on apache/nifi-api#26 (NIFI-15155)
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation