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: make libretranslate works with ipv6 #596

Merged
merged 3 commits into from Mar 6, 2024

Conversation

cyrinux
Copy link
Contributor

@cyrinux cyrinux commented Mar 6, 2024

This should still works with ipv4 then. Tested on a only ipv6 k8s cluster.

@cyrinux cyrinux changed the title fix: make libretranslate with ipv6 fix: make libretranslate works with ipv6 Mar 6, 2024
@pierotofy
Copy link
Member

pierotofy commented Mar 6, 2024

Thanks for the PR @cyrinux !

This will break access to ipv4 however; (0.0.0.0 will no longer be accessible).

Waitress has a special "*" address for binding to both ipv4 and ipv6 (see https://docs.pylonsproject.org/projects/waitress/en/stable/usage.html).

"*" will however not work when --debug is used (see main.py:219).

I would add a check near that line where "if debug and host == '*': then host = '0.0.0.0'".

@cyrinux
Copy link
Contributor Author

cyrinux commented Mar 6, 2024

Hi @pierotofy , Ok, I will try to fix in that way.

As far this is working, I though it would be ok too in python:
image

What about the healthcheck so? We check both 127.0.0.1 and ::1 ? I will maybe just use http://localhost:5000

@cyrinux
Copy link
Contributor Author

cyrinux commented Mar 6, 2024

I push a new commit update, and choose to convert "*" to "::" for the debug mode, as "::" will make listen on both ipv4 and ipv6, is it ok for you @pierotofy ?

@pierotofy
Copy link
Member

Nice, yes that works.

I think ENTRYPOINT [ "./venv/bin/libretranslate", "--host", "::" ] should be changed to ENTRYPOINT [ "./venv/bin/libretranslate", "--host", "*" ], after which this can be merged.

@cyrinux
Copy link
Contributor Author

cyrinux commented Mar 6, 2024

Nice, yes that works.

I think ENTRYPOINT [ "./venv/bin/libretranslate", "--host", "::" ] should be changed to ENTRYPOINT [ "./venv/bin/libretranslate", "--host", "*" ], after which this can be merged.

Thanks, sorry, I forgot to commit them. I just test a last time, on my ipv6 only cluster. Its OK for me. I will be happy if you merge and publish a new tag then 👍🏻 🤗

@pierotofy pierotofy merged commit 6ae0091 into LibreTranslate:main Mar 6, 2024
4 checks passed
@pierotofy
Copy link
Member

Looks great, thanks!

@pierotofy
Copy link
Member

https://github.com/LibreTranslate/LibreTranslate/releases/tag/v1.5.6

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.

None yet

2 participants