-
Notifications
You must be signed in to change notification settings - Fork 121
Re-introduce public and private ip-getting, add LSF test #12269
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR re-introduces the ability to select between private and public IP addresses for network communication, defaulting to public IPs. The change reverts from a simpler socket-based IP detection to an approach that examines all network interfaces and classifies them as loopback, private, or public.
Key Changes
- Refactored
get_ip_address()to support optional prioritization of private vs. public IPs - Added comprehensive unit tests for IP address selection logic across various network configurations
- Added integration test for ZMQ server communication over TCP using drivers (LocalDriver and LsfDriver)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/ert/shared/net_utils.py |
Replaced socket-based IP detection with psutil-based interface enumeration; added prioritize_private parameter to get_ip_address() |
tests/ert/unit_tests/shared/test_net_utils.py |
Added parameterized unit tests covering IP selection scenarios with mock network interfaces |
tests/ert/unit_tests/scheduler/test_that_fm_dispatch_can_communicate_with_zmq_server.py |
New integration test verifying fm_dispatch communication with ZMQ server over TCP on LSF/local drivers |
Comments suppressed due to low confidence (1)
src/ert/shared/net_utils.py:1
- Typo in parameter name: 'psutils_if_addrs' should be 'psutil_if_addrs' (missing 'i'). The module is called 'psutil', not 'psutils'. This appears in the test parameter name
mock_psutils_if_addrson line 365 as well.
import ipaddress
tests/ert/unit_tests/scheduler/test_that_fm_dispatch_can_communicate_with_zmq_server.py
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12269 +/- ##
==========================================
- Coverage 90.64% 90.63% -0.01%
==========================================
Files 434 433 -1
Lines 29015 29028 +13
==========================================
+ Hits 26300 26310 +10
- Misses 2715 2718 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #12269 will not alter performanceComparing Summary
|
- Add optional flag to prioritize private IPs, default false - Update tests for correct ip address fetching This reverts commit 75b190d.
f32bfb6 to
d792081
Compare
1f94b97 to
e15161e
Compare
Test will time out if ip is not reachable
307e55f to
bb34e64
Compare
7af284f to
a7784ef
Compare
a7784ef to
248baa0
Compare
Issue
Resolves #12236
Approach
Revert back the IP selection with private and public, but use public by default
This paves way for a configurable site-config variable.
Adds a test that can be run on LSF queue cluster, that will fail (timeout) if network used is not reachable
git rebase -i main --exec 'just rapid-tests')When applicable