Skip to content
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

fix: Restore default tile server port to 5005 #529

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

achou11
Copy link
Member

@achou11 achou11 commented Sep 23, 2021

Fixes #527

The default tile server port was changed to 5100 in 5467c19 :

tileport: 5100,

Not entirely sure if that was intentional but for the sake of restoring previously working behavior and avoiding updates to programmatic documentation, this commit restores the default port to 5005

@gmaclennan
Copy link
Member

Oh, good find! I changed this because in #465 I switched to using get-port, which gets the next available port if the requested port is in-use (as opposed to just crashing, which is what happens now). I switched it to 5100 to leave "room" for the Mapeo Server port, which defaults to 5000 (get-port keeps incrementing by 1 until it finds a free port).

As such, I think this change is fine and the best fix. There are two edge-cases:

  • If the user's machine has something running on ports 5000, 5001, 5002, 5003 and 5004, then both the Mapeo Server and Tile Server will try to run on port 5005 (because we currently get free ports in parallel and do not lock them until we start the server listening). This is very unlikely. The consequence will be a fatal error in the app (because the server will not be able to start)
  • Not a consequence of this change, but a result of the fix: Fix Mapeo window randomly not opening at startup #465 refactor: if port 5005 is in-use on the users machine, then the tile server will run on the next available port (e.g. 5006), so setting a custom background to use 5005 will not work.

I don't think these are significant to worry about at this stage, and when we refactor some of this (run a single server on a single port; run a map server with UI integration to avoid typing in template URLs) then this will be fixed in a more complete way.

@achou11
Copy link
Member Author

achou11 commented Sep 24, 2021

@gmaclennan makes sense! I figured there was a strong reason to switch the ports. This wouldn't be as much of an issue if the documentation for programs was aligned with the change, but given the unlikelihood of the edge cases you mentioned, I'll just merge this and we can address later.

Also love how I did a whole git bisect (I think with ~10 steps) to find this one line 😅

@achou11 achou11 merged commit e1b5cd6 into master Sep 24, 2021
@achou11 achou11 deleted the ac/527/territory-custom-port branch September 24, 2021 14:48
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.

Can't get Custom background maps to appear in Territory view
2 participants