Skip to content

NIFI-11603: Removed NetworkUtils.getAvailableUdpPort, NetworkUtils.ge…#7299

Closed
markap14 wants to merge 3 commits intoapache:mainfrom
markap14:NIFI-11603
Closed

NIFI-11603: Removed NetworkUtils.getAvailableUdpPort, NetworkUtils.ge…#7299
markap14 wants to merge 3 commits intoapache:mainfrom
markap14:NIFI-11603

Conversation

@markap14
Copy link
Contributor

@markap14 markap14 commented May 25, 2023

…tAvailableTcpPort and NetworkUtils.getAvailablePort as well as NetworkUtils.isListening

Summary

NIFI-00000

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

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 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

@markap14
NIFI-11603: Removed NetworkUtils.getAvailableUdpPort, NetworkUtils.ge…
fafd279 
…tAvailableTcpPort and NetworkUtils.getAvailablePort as well as NetworkUtils.isListening
@joewitt
Copy link
Contributor

joewitt commented May 25, 2023

building on a local osx and linux set of machines now.

@joewitt
Copy link
Contributor

joewitt commented May 25, 2023

with -Pinclude-grpc it appears some bits were missed

[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:testCompile (default-testCompile) on project nifi-grpc-processors: Compilation failure: Compilation failure:
[ERROR] /development/code/nifi.git/nifi-nar-bundles/nifi-grpc-bundle/nifi-grpc-processors/src/test/java/org/apache/nifi/processors/grpc/TestListenGRPC.java:[78,37] error: cannot find symbol
[ERROR] symbol: method getAvailableTcpPort()
[ERROR] location: class NetworkUtils
[ERROR] /development/code/nifi.git/nifi-nar-bundles/nifi-grpc-bundle/nifi-grpc-processors/src/test/java/org/apache/nifi/processors/grpc/TestListenGRPC.java:[90,37] error: cannot find symbol
[ERROR] symbol: method getAvailableTcpPort()
[ERROR] location: class NetworkUtils
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn -rf :nifi-grpc-processors

@markap14
Copy link
Contributor Author

markap14 commented May 25, 2023

Thanks @joewitt missed that one since I didn't build with the profile

exceptionfactory
exceptionfactory reviewed May 31, 2023
View reviewed changes
Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing these issues @markap14, the approach looks good. I only noticed two minor things, one was an unused member variable and the other was a hard-coded client port number, otherwise this looks good.

...nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenSyslog.java Outdated
private volatile SyslogParser parser;
private volatile BlockingQueue<ByteArrayMessage> syslogEvents = new LinkedBlockingQueue<>();
private volatile byte[] messageDemarcatorBytes; //it is only the array reference that is volatile - not the contents.
private volatile int listeningPort;
Copy link
Contributor

@exceptionfactory exceptionfactory May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value does not appear to be assigned.

...ifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestListenUDP.java Outdated
@Test
public void testWithSendingHostAndPortSameAsSender() throws IOException, InterruptedException {
final Integer sendingPort = NetworkUtils.getAvailableUdpPort();
final int sendingPort = 27911;
Copy link
Contributor

@exceptionfactory exceptionfactory May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this hard-coded port result in intermittent failures? Can it be set to 0 for constructing the DatagramSocket?

Copy link
Contributor Author

@markap14 markap14 May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I meant to come back to that one, as I wasn't 100% sure what was happening there. Will address, though.

exceptionfactory
exceptionfactory approved these changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates @markap14, the latest version looks good! +1 merging

exceptionfactory pushed a commit that referenced this pull request Jun 1, 2023
@markap14 @exceptionfactory
NIFI-11603 Refactored Socket-based tests to use Dynamic Ports
e75d78b 
- Removed NetworkUtils methods for getting available ports
- Updated Socket-based components to support using 0 to listen on a random available port for improved test reliability

This closes #7299

Signed-off-by: David Handermann <exceptionfactory@apache.org>
(cherry picked from commit 5081166)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@exceptionfactory exceptionfactory exceptionfactory approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@markap14 @joewitt @exceptionfactory