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

feat(i18n): Watch beabee content for locale changes #3

Merged
merged 9 commits into from
Feb 7, 2024

Conversation

JumpLink
Copy link
Collaborator

@JumpLink JumpLink commented Jan 12, 2024

Watch beabee content for locale changes by pulling the API.

  • I use this to sync the telegram-bot language as soon as they are changed in beabee.
  • I also added a waitForUrl util method to wait for the beabee api on bootstrapping the bot.

@JumpLink JumpLink requested a review from wpf500 January 12, 2024 16:09
…to the latest tag in the beabee/telegram-bot repository
fix(telegram.service.ts): import readJson function from utils/file.ts to parse JSONC files
feat(telegram.service.ts): add printInfo method to log package name, version, and bot username on startup
fix(file.ts): use parseJsonc function from deps.ts to parse JSONC files instead of JSON.parse
Copy link
Member

@wpf500 wpf500 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the attempt to make this as pure as possible but I think this might be a bit over engineered 😛

In practice an instance's language basically never changes once it's been set up, and this solution generates a lot of API requests. I think it would be better just to have an endpoint we can push to.

This actually already exists for the intended use case (watching for locale changes)
https://github.com/beabee-communityrm/beabee/blob/master/src/core/services/OptionsService.ts#L123

The services all share an internal network in their Docker stack which they can use to communicate. Obviously it's very hacky at the moment, I'd love for this to just broadcast to the internal network, but I think it's better than polling the API so much. What do you think?

@JumpLink
Copy link
Collaborator Author

thanks that's better, I'll implement it

@JumpLink
Copy link
Collaborator Author

JumpLink commented Jan 18, 2024

Do I just need to add the telegram bot service to the notify list and implement a reload controller in the bot to refetch the content?

@wpf500
Copy link
Member

wpf500 commented Jan 18, 2024

Yeah you can see the very simple route that gets created here for the legacy app, API and webook app:

https://github.com/beabee-communityrm/beabee/blob/master/src/core/server.ts#L13

It basically creates an internal HTTP server that listens to :4000, I'd do the same for the telegram bot too. Then just needs to refetch the content when it gets that signal.

Ideally we wouldn't reference the telegram bot in the core app but it might be the simplest thing to do for now, just behind an env var so it can be enabled/disabled.

You could do a quick bit of research into what it would take to create an internal broadcast messaging service like RabbitMQ that services can subscribe to, but if it's not really simple them just hardcode it for now.

@wpf500
Copy link
Member

wpf500 commented Jan 18, 2024

Or I know that people often use Redis for this sort of thing too: https://redis.com/solutions/use-cases/messaging/

@wpf500
Copy link
Member

wpf500 commented Jan 18, 2024

But just to emphasise: this is not a priority right now and hardcoding is also okay 🙃

@JumpLink
Copy link
Collaborator Author

Okay, since we don't plan for the bot to have its own interface anyway, we can simply set the port of the existing web server to 4000 and use it for that

@JumpLink
Copy link
Collaborator Author

JumpLink commented Jan 18, 2024

As long as this communication is really only internal and cannot be intercepted, I think this is a good and simple solution. But maybe we should change http to https

@wpf500
Copy link
Member

wpf500 commented Jan 18, 2024

Hmm I don't think you could use HTTPS here as app, api_app, etc. are internal names resolved by Docker's DNS so you'd only be able generate self signed certs in any case.

I guess in theory someone could accidentally expose :4000 to the outside world. Clients using our hosted solution will be fine but maybe we should add a secret or something just to be safe?

@JumpLink
Copy link
Collaborator Author

JumpLink commented Feb 7, 2024

@wpf500 I have now also implemented the NetworkCommunicatorService here and replaced the pulling with pushing. It may make sense to outsource the NetworkCommunicatorService in order to be able to use it in different services, as we already have two implementations, this one uses fetch and would therefore theoretically be platform-independent

@JumpLink JumpLink merged commit 7162206 into main Feb 7, 2024
1 check failed
@JumpLink JumpLink deleted the content-polling branch February 7, 2024 18:13
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.

2 participants