cmj2002 / warp-docker

Run Cloudflare WARP in Docker.
GNU General Public License v3.0
256 stars 84 forks source link

add network check #11

Closed luckyops closed 10 months ago

cmj2002 commented 10 months ago

Thank you for your contribution and your time spent on this Pull Request. I've reviewed your changes and I have some feedback I'd like to share:

  1. The introducing of Python seems unnecessary for your purpose. It significantly increases the Docker image size, which could be a concern for deployment and runtime efficiency. I'd suggest migrating it to shell script, if possible.

  2. The feature implemented by self_check is already covered by the existing docker healthcheck. In addition, when the check fails, user will be prompted with a menu instead of exiting with an error, which will cause the automatic restart of Docker to not work.

  3. The feature to choose a custom endpoint doesn't necessarily need to be implemented within the container. Instead, it could be run directly on the host machine and then passed to the container via environment variables. We generally avoid running interactive programs within containers in production.

In light of the above points, a more suitable approach might be to run your Python script on the host machine, and pass the custom endpoint into the container via environment variables. The container should then include the necessary warp-cli set-custom-endpoint code.

I appreciate your understanding and look forward to your response. Please let me know if you have any questions or need further clarification.