Skip to content

Fix binding when user sets HTEx worker ports manually #3838

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

Merged
merged 3 commits into from
May 24, 2025

Conversation

WardLT
Copy link
Contributor

@WardLT WardLT commented Apr 7, 2025

Description

Fixes the creation of the TCP url when the ports used by HTEx workers are fixed.

The tcp_url helper function from addresses.py was only appending the port number when the address was not "*".

Changed Behaviour

HTEx works with the ports fixed. Not sure when it was broken

I added a unit test which shows that the current code fails when launching an HTEx interchange with user-set ports.

Fixes

None (yet? should I make one?)

Type of change

  • Bug fix

@WardLT WardLT force-pushed the fix-zmq-fixed-bindings branch 3 times, most recently from 450f44b to 41dd76c Compare April 7, 2025 01:04
@WardLT WardLT marked this pull request as ready for review April 7, 2025 01:57
@WardLT
Copy link
Contributor Author

WardLT commented Apr 7, 2025

Might I suggest @yadudoc for the review? I altered a function he wrote a few months ago

@benclifford benclifford requested a review from yadudoc April 7, 2025 07:49
@benclifford
Copy link
Collaborator

you might add a case into parsl/tests/unit/test_address.py which more directly tests the tcp_url function

Copy link
Collaborator

@khk-globus khk-globus 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 adding the test. We're working through a build error at the moment, but I'll merge this once that gets sorted. Thanks!

@khk-globus khk-globus force-pushed the fix-zmq-fixed-bindings branch from ff7fc22 to 6fe7db2 Compare May 24, 2025 03:36
@khk-globus khk-globus enabled auto-merge May 24, 2025 03:37
@khk-globus khk-globus added this pull request to the merge queue May 24, 2025
Merged via the queue into Parsl:master with commit f2e5f5f May 24, 2025
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants