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

Rewrite Docker image to function with current HEAD and upstream wego #614

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jinnatar
Copy link
Contributor

  • Use debian buster based images due to the terrible availability of
    alpine compatible wheels and ansi2html failing under alpine.
  • Automate fetching of airport.dat & GeoLite2 but an API key must be
    passed at image build time
  • Also include upstream wego in the image since it supports more
    backends that still provide personal API keys.
  • Rewrite wego compatibility back to upstream wego. Ideally there should
    be a config option to use the fork or upstream and to change the wego
    param --location/--city based on it.

- Use debian buster based images due to the terrible availability of
alpine compatible wheels and ansi2html failing under alpine.
- Automate fetching of airport.dat & GeoLite2 but an API key must be
passed at image build time
- Also include upstream wego in the image since it supports more
backends that still provide personal API keys.
- Rewrite wego compatibility back to upstream wego. Ideally there should
be a config option to use the fork or upstream and to change the wego
param --location/--city based on it.
@chubin
Copy link
Owner

chubin commented Oct 31, 2021

Very cool contribution! Thank you.

Several questions:

  • Have you fully disabled v3, but it is currently in use (https://v3.wttr.in); yes, you are right, some code is missing in the master branch, but maybe we should better add this code, than disable all v3-related things?
  • Agree that it is great to have wego compatibility, but currently we use we-lang (the wttr.in for of wego, that is quite diverged from it already); maybe we need to support both of them. If I understand it right, I just should add --location option?
  • ansi2html usage will be hopefully dropped soon. Do you think that it is still worth it to switch to Debian in this case?

@jinnatar
Copy link
Contributor Author

jinnatar commented Nov 3, 2021

  • I only ripped out the bare minimum to avoid initializing v3 structures that don't exist in the current branch.
  • Adding would be better yes, but I'd recommend doing that in a separate v3 branch so that the main branch is self-contained and functional, instead of somewhere half between v2 & v3.
  • I can't remember all the details anymore on the wego compatibility, but the fork was quite heavily focused on the one API available to you, and since the API space keeps on evolving and depracating itself, it can become impossible to get API keys for any single API. Thus an option to use upstream wego should probably remain an option.
  • I would still recommend a Debian base since image builds are significantly faster on it due to available wheels for glibc. The image size difference isn't all that much either.

@dhuys
Copy link

dhuys commented Sep 27, 2022

Bumping this.. I would like to use docker but it currently seems a bit outdated. Is this PR still valid?

@dhuys
Copy link

dhuys commented Sep 27, 2022

@chubin

@jinnatar
Copy link
Contributor Author

Let me do a thorough check later if I can sanely rebase this up to date, but pretty sure this is what I'm running in prod.

@jinnatar
Copy link
Contributor Author

jinnatar commented Oct 9, 2022

Having dug into this again, there's a lot of work to do unfortunately. The codebase has had large changes again without updating the Dockerfile at all, so it seems @chubin is not running their prod instance on Docker since the codebase is anything but hermetic and relies on static paths to resources such as airports.dat.

I'll try to create side by side a new PR that adds in the necessary overrides to make the codebase work in a hermetic environment and then drag the Dockerfile up to speed to run said hermetic environment.

Ultimately though this will be a futile effort in the long run as long as Igor runs his production in a non-hermetic way and evolves the codebase with that in mind and without testing the code on Docker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants