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

osrm: Use the host even for local servers #822

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

tahini
Copy link
Collaborator

@tahini tahini commented Jan 9, 2024

fixes #821

This uses the value set in the mode config's host field even for local servers. This allows to specify a specific URL on which to join the server, for example an ipv4 url where 'localhost' would have resolved to an ipv6 url, or simply a named URL for the local server.

@tahini tahini requested a review from greenscientist January 9, 2024 15:02
Copy link
Collaborator

@greenscientist greenscientist left a comment

Choose a reason for hiding this comment

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

Shouldn't we add the host to the restart call a bit after this?

@tahini
Copy link
Collaborator Author

tahini commented Jan 9, 2024

Shouldn't we add the host to the restart call a bit after this?

No, the host here is used locally for the mode registry, but is not used for the osrm-routed's host parameter (-i), so it is not required in the start/restart commands.

@greenscientist
Copy link
Collaborator

ok, yeah I see.
Please add a bit of comments to explain why we need the hostname even when we use the autostart.

fixes chairemobilite#821

This uses the value set in the mode config's host field even for local
servers. This allows to specify a specific URL on which to join the
server, for example an ipv4 url where 'localhost' would have resolved to
an ipv6 url, or simply a named URL for the local server.
@tahini
Copy link
Collaborator Author

tahini commented Jan 9, 2024

It's in the commit message, but I added it in the code too

@tahini tahini merged commit bde70a2 into chairemobilite:main Jan 9, 2024
4 checks passed
@tahini tahini deleted the fixOsrmHost branch January 9, 2024 16:20
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.

osrm: local osrm servers listen on ipv4, but transition may look for ipv6
2 participants