Skip to content

Conversation

@nattsw
Copy link
Contributor

@nattsw nattsw commented Nov 20, 2024

Currently, upon post serialization, we spawn a
new job to detect the language of the post. This
could potentially flood sidekiq on larger sites.

This PR uses redis to keep track of posts that
need language detection, then schedules them in
a 5-minute-ly job, with a maximum of 1000 posts
at a time.

Note for the reviewer: This plugin doesn't seem to be using autoloading as it should yet, but that will come in a separate PR.

Currently, upon post serialization, we spawn a
new job to detect the language of the post. This
could potentially flood sidekiq on larger sites.

This PR uses redis to keep track of posts that
need language detection, then schedules them in
a 5-minute-ly job, with a maximum of 1000 posts
at a time.
@nattsw nattsw force-pushed the use-scheduled-batch-job branch from d35ea01 to add68d4 Compare November 20, 2024 05:11
@Drenmi
Copy link
Contributor

Drenmi commented Nov 20, 2024

What are the consequences if we build up a bit of a backlog in this queue? Just need to wait a bit longer for the translations to happen?

@nattsw
Copy link
Contributor Author

nattsw commented Nov 20, 2024

@Drenmi for now I believe waiting is fine for language to be detected. Note that this PR involves language detection, not the translation itself.

The current behaviour also doesn't instantly update the post, since it is an asynchronous job triggered during serialization which adds custom fields. Actually I lied -- adding the post custom field does allow the post to be updated through the magic of message bus. The 🌐 icon appears after the job is executed.

@nattsw
Copy link
Contributor Author

nattsw commented Nov 20, 2024

@jjaffeux I've incorporated your suggestions :)

@nattsw nattsw merged commit 8fce768 into main Nov 20, 2024
3 checks passed
@nattsw nattsw deleted the use-scheduled-batch-job branch November 20, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants