freiheit / discord_feedbot

Moved to https://gitlab.com/ffreiheit/discord_feedbot
MIT License
82 stars 28 forks source link

feat(Docker): Fix Dockerfile and add generation of config file from json file and env vars #152

Closed ScriptSathi closed 2 years ago

ScriptSathi commented 2 years ago

An error happened when i tried to build the image using alpine 3.16

ERROR: unable to select packages:
  libressl2.4-libssl (no such package):
    required by: world[libressl2.4-libssl]
The command '/bin/sh -c apk add --update --no-cache         ca-certificates         libressl2.4-libssl      python3 &&  python3 -m ensurepip &&     rm -r /usr/lib/python*/ensurepip && pip3 install --upgrade      pip         setuptools &&   pip3 install        aiohttp         discord.py      feedparser      html2text       python-dateutil         pytz        requests        websockets      ws4py       &&  rm -r /root/.cache' returned a non-zero code: 1

After alpine v3.5 the libressl2.4-libssl no more exist and the generic name changed to libressl-dev

ScriptSathi commented 2 years ago

Btw what do you think about changing the image to python:3.10.5-alpine3.16 and just calling this bellow ?

COPY requirements.txt ./
RUN pip install --no-cache-dir -r requirements.txt
freiheit commented 2 years ago

Btw what do you think about changing the image to python:3.10.5-alpine3.16 and just calling this bellow ?

COPY requirements.txt ./
RUN pip install --no-cache-dir -r requirements.txt

I don't personally use the docker stuff. And looking at Dockerfile now, I can see its overdue for some updates. That seems like a good improvement. Make that change, test it, let me know it worked, and I'll merge it.

ScriptSathi commented 2 years ago

I think this project is broken since a while. I would like to make an easy usage with docker and post it on docker hub.I've almost done all the docker part for it but feed2discord.py is not working.

this line make the program broken since maybe you migrate it in async await ?

 loop.run_until_complete(client.login(MAIN.get("login_token")))

client.login should be async await concidering the documentation.

What version of this bot (latest commit) are you running in production ?

freiheit commented 2 years ago

@ScriptSathi It's entirely possible that the Dockerfile has been broken for a while... Somebody else contributed it a while back, but I don't use it and haven't really looked at it.

If you want to publish it on Docker Hub, maybe we could collaborate on giving you "code ownership" of the Dockerfile and related pieces and set up this repo to automatically publish "releases" to Docker Hub?

Let's see... I was running off of dddb60f with a couple local experimental changes... I went ahead and committed some stuff and pushed it up and latest commit (7ad2af8) is exactly what I'm running, and it looks fine...

Can't really await client.login() inside of main() because main() isn't (and can't be) async (as far as I know). loop.run_until_complete() is a weird case. IIRC, I pulled that from a discord.py example for background tasks, but the docs have updated quite a bit and I should probably refactor the "startup" to look more like https://github.com/Rapptz/discord.py/blob/master/examples/background_task_asyncio.py

What I'm actually running the production bot on is:

ScriptSathi commented 2 years ago

I tried to replace client.logiǹ toclient.run` before my last comment but i didn't get much further.Though i'll be glad to help with the docker part (as long as i know it worked lol) but i've no idea how to automate the CD part through github. Btw i choose to fix version on requirement.txt because i was afraid of some big changelogs on some libraires that could potentially break some stuff. Even if we update it once a year it's better than nothing

Anyway thanks for your time, i'll make changes on README for the "how to use" docker part it should be really easy after this.

Just to tell you about a small changes i've done for docker compatibility is to remove CHANNELS usage from feed2discord.local.ini because it'll be too complicated to manage it with adding the name in the feeds. So to be simple feeds would look like

[ednews]
channels = <your channel id here>
feed_url = https://community.elitedangerous.com/taxonomy/term/6/feed
fields = link,**title**,_published_,description

But i did'nt check if feed2discord.py allow this usage.

And finnaly i think i missed some features but i think i got the most important onces ^^

freiheit commented 2 years ago

I tried to replace client.logiǹ toclient.run` before my last comment but i didn't get much further.

I'll try to work on this soon... At least a few days before I'll have a chance to get to it, though.

I want to reorganize it to be a closer match to: https://github.com/Rapptz/discord.py/blob/master/examples/background_task_asyncio.py

Though i'll be glad to help with the docker part (as long as i know it worked lol) but i've no idea how to automate the CD part through github.

There's a pre-packaged GitHub workflow for publishing Docker images to a hub. I've done a fair amount of CI/CD stuff via GitLab before, and most of these really aren't that hard to figure out.

Btw i choose to fix version on requirement.txt because i was afraid of some big changelogs on some libraires that could potentially break some stuff. Even if we update it once a year it's better than nothing

FYI, I do have "RenovateBot" watching for updates on dependencies in requirements.txt. #95 has an overview (expand the "pip_requirements" chunk), and a recent example is #137.

Anyway thanks for your time, i'll make changes on README for the "how to use" docker part it should be really easy after this.

Yes, please. :)

Just to tell you about a small changes i've done for docker compatibility is to remove CHANNELS usage from feed2discord.local.ini because it'll be too complicated to manage it with adding the name in the feeds. So to be simple feeds would look like [...] But i did'nt check if feed2discord.py allow this usage.

Hmmm... I don't want to abandon CHANNELS, since it's pretty essential for managing larger configurations (like if many feeds go to multiple rooms).

The code currently strictly requires CHANNELS, but I think it'd be easy to change that...

I was thinking "look at the channels item and use it as the channel_id if it's numeric", but looking at the code I think I could accomplish it simply by changing

        channel_id = config["CHANNELS"].getint(key)

to

        channel_id = config["CHANNELS"].getint(key,key)

That way it would fall back to directly using the id from channels= if no match was found in [CHANNELS].

Note: I actually handle the "complicated" case in newfeed.py already: it shows most recent feed entry, prompts for fields (with some typical examples printed out), asks for a room name, creates the room, and edits the config. And newfeed.sh wraps around that with the addition of some automatic git commit/push and restarting the service.

freiheit commented 2 years ago

This Pull Request has gotten big enough that it's hard to evaluate...

Any chance you could pare this PR down to just the essential changes for Docker and have separate PRs for other things?

ScriptSathi commented 2 years ago

Yeah ok, i'll just remove the changes on feed2discord.py and keep current changes. I continue to work on it because it wasn't working before i started the refacto so i kept all the changes. But it's true it's a bit confusing

EDIT:

Concidering your latest changing on documentation i'll put a link to the DOCKER.md and it should be fine ^^

ScriptSathi commented 2 years ago

I rebase the branch from your latest changes on README and other .md files and moved my commits on feed2discord.py to another branch for an easy usage. So this branch is done i should not put other commit. Only if you suggest changes ofc