Skip to content

feat: Automatically assign ports when available#96

Merged
ShrimpCryptid merged 4 commits into
mainfrom
feat/automatic-port-assignment
Oct 24, 2025
Merged

feat: Automatically assign ports when available#96
ShrimpCryptid merged 4 commits into
mainfrom
feat/automatic-port-assignment

Conversation

@ShrimpCryptid
Copy link
Copy Markdown
Contributor

@ShrimpCryptid ShrimpCryptid commented Oct 13, 2025

Problem

Change by @alexkhang18 that I'm PRing on his behalf, with a few minor changes. This change allows the tfe-open command to automatically assign new port numbers if the current ones are occupied.

Estimated review size: tiny, 5 minutes

Testing

  1. Initialize a virtual environment if needed.
  2. Run pip install -e '.[dev]'
  3. Run tfe-open documentation/example_dataset. A new browser window should open.
  4. Repeat the above steps in another window.

Video talk-through (🔊):

2025-10-13.15-57-42.mp4

@ShrimpCryptid ShrimpCryptid self-assigned this Oct 13, 2025
@ShrimpCryptid ShrimpCryptid added the new feature New feature or request label Oct 13, 2025
@ShrimpCryptid ShrimpCryptid marked this pull request as ready for review October 13, 2025 23:00
@ShrimpCryptid ShrimpCryptid requested a review from a team as a code owner October 13, 2025 23:00
@ShrimpCryptid ShrimpCryptid requested review from ascibisz, meganrm and rugeli and removed request for a team and meganrm October 13, 2025 23:00
Copy link
Copy Markdown

@rugeli rugeli left a comment

Choose a reason for hiding this comment

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

it's a handy feature! left a non-blocking note

Comment thread colorizer_data/bin/tfe_open.py Outdated
@ShrimpCryptid ShrimpCryptid requested a review from rugeli October 23, 2025 22:24
Comment thread colorizer_data/bin/tfe_open.py Outdated


def get_available_port(default_port):
def get_available_ports(default_ports: List[int]) -> List[int]:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

smart choice making it recursive! There is a recursion depth limitation with python but we're unlikely to hit it with just few ports so it's totally fine.
Although another thought I have, return a list of tuples like [(socket, port), ...] instead of just port numbers, then s.close() to close the sockets only after the servers running(or on error). This way should be safer because the ports stay reserved until we successfully started the servers on them

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a good idea, thank you, Ruge!

@ShrimpCryptid ShrimpCryptid requested a review from rugeli October 23, 2025 23:46
Copy link
Copy Markdown

@rugeli rugeli left a comment

Choose a reason for hiding this comment

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

Looks great!

@ShrimpCryptid ShrimpCryptid merged commit 021ff8c into main Oct 24, 2025
1 check passed
@ShrimpCryptid ShrimpCryptid deleted the feat/automatic-port-assignment branch October 24, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants