Uniformization of messages publication on discord

[aidanjungo]

General purpose script to publish message automatically on a channel of Tournesol Discord server.

[aidanjungo]

@amatissart @GresilleSiffle before I go further, could you check the main function? A few question from my side:

• Is it the correct place to store this function
• I am not sure the "if name == "main":" is useful, especially if we add a Django management command
• Could we import Django settings from outside Django (for Ansible)?

[GresilleSiffle]

Hello @aidanjungo

Thanks for the ping : )

Is it the correct place to store this function

I'd say yes and no at the same time. Yes because the scripts folder looks attractive thanks to its generic name, and your Python module can be used as a script and as a imported library.

I think I prefer to say no, because currently the scripts folder is not used by any Django apps and can be safely removed in production (in fact I didn't test it so it's just a supposition) without making the back end crash.

For general purpose Python libraries, we have several options:

# discord as a top-level folder
backend/discord/api.py
backend/discord/send.py

# discord as part of a top-level generic library (i.e. that can be used by any other Python modules)
backend/lib/discord/api.py
backend/lib/discord/send.py

(Note that send.py is just an example to show it's possible to split the module into a lib and a script using the lib, in our case we might want a Django mgmt command instead.)

As your code is not coupled to any existing app. I prefer backend/lib/discord over backend/tournesol/lib/discord.

As the number of top-level folders can grow, I prefer backend/lib/discord over backend/discord to encourage developers to write re-usable lib without dependency to the existing apps (core, tournesol, etc.).

What do you think @amatissart ?

I am not sure the "if name == "main":" is useful, especially if we add a Django management command

Indeed it's better to use a Django management command in our case, to benefits from all of its included features like the option parsing, the log, etc.

Could we import Django settings from outside Django (for Ansible)?

I'm not sure, but I think we don't need it. I think we Django settings should be configured by Ansible, like we currently do, and not the opposite to avoid introducing a new back and forth behaviour.

---

What's the goal of this PR exactly?

• create a discord lib?
• create a discord lib + make it configurable during the deployment?
• create a discord lib + make it configurable during the deployment + make the twitterbot app use it?

[amatissart]

backend/lib/discord looks good to me, to make the distinction with "scripts" used for development purposed or maintenance operations.

One thing to keep in mind: Django settings (imported via from django.conf import settings) need to be used in the context of a django app. As is, I don't think it would work correctly as a standalone script. But creating a new management command, while keeping the possibility to use the library in the context of any django app would probably make sense.

And the management command could also be used to replace the existing webhooks used in ExecStopPost= to alert about service failures (and avoid duplication of alert scripts). For example here: https://github.com/tournesol-app/tournesol/blob/53c645b7ed22589937784dc4d10d0186394f9c66/infra/ansible/roles/django/templates/ml-train.service.j2#L12 and in other similar scripts used in export-backups.service, tournesol-api-cleartokens.service and tournesol-api-delete-inactive-users.service, as well as in Ansible handlers notifying about new deployments.

[aidanjungo]

Thanks @GresilleSiffle and @amatissart for the answers. I move the script, now I wonder where I should put the management command?

• backend/lib/management/
• backend/lib/discord/management/
• backend/core/management/

[aidanjungo]

Task remaining:

• [x] Add the secret names in the bash scripts (deploy..., forget..., get-vm...)
• [x] Replace existing scripts by the new management command
• [x] Call the the management command (or the function directly) after a tweet has been posted

@amatissart Could you check what I have done so far, so I did not change everything before I am sure it is how we want to proceed?

[GresilleSiffle]

Hello!

I'll review this afternoon.

Few things I'll probably tweak and push:

• [x] publish_on_discord could be moved outside of the management command, as it used elsewhere in the project
• [x] publish_on_discord could take the webhook as argument to be more flexible
• [x] lib/management/commands doesn't seem to be used, and seem very specific, could be deleted (a lib dedicated to management commands? an not other modules?)