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(condo): INFRA-300 Message prioritization 📧📈 #4591

Merged
merged 6 commits into from Apr 11, 2024

Conversation

SavelevMatthew
Copy link
Member

No description provided.

@SavelevMatthew SavelevMatthew added 👶 small Easy to review changes up to 50 lines of code ✋🙂 Review please Comments are resolved, take a look, please 😎 Cool labels Apr 9, 2024
@SavelevMatthew SavelevMatthew marked this pull request as draft April 9, 2024 10:57
* @returns {'medium' | 'high' | 'low'} condo task queue
*/
function getMessageQueue ({ type }) {
// TODO(VKislov): separate queue for VoIP here?
Copy link
Member

Choose a reason for hiding this comment

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

Jira task?

@SavelevMatthew SavelevMatthew force-pushed the feat/condo/INFRA-300/send-message-priority branch from f7a9a05 to faac6d6 Compare April 9, 2024 11:27
@SavelevMatthew SavelevMatthew marked this pull request as ready for review April 9, 2024 11:28
Copy link
Member

@AleX83Xpert AleX83Xpert left a comment

Choose a reason for hiding this comment

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

Why we can not use bull's priorities?

@sitozzz
Copy link
Member

sitozzz commented Apr 9, 2024

Why we can not use bull's priorities?

Because of how this mechanism works under the hood. In short, this is due to the fact that priority in the queue only works with redis. Also that's really expensive operation for redis instance.

@SavelevMatthew SavelevMatthew force-pushed the feat/condo/INFRA-300/send-message-priority branch from 911ad5a to 7b31e2d Compare April 11, 2024 08:27
@@ -100,7 +100,9 @@ function removeCronTask (name, cron, opts = {}) {
REMOVE_CRON_TASKS.push([name, taskOpts])
}

async function _scheduleRemoteTask (name, preparedArgs, preparedOpts, queue = DEFAULT_QUEUE_NAME) {
async function _scheduleRemoteTask (name, preparedArgs, preparedOpts, queueName = DEFAULT_QUEUE_NAME) {
const queue = TASK_QUEUE_REMAPPING.hasOwnProperty(queueName) ? TASK_QUEUE_REMAPPING[queueName] : queueName
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const queue = TASK_QUEUE_REMAPPING.hasOwnProperty(queueName) ? TASK_QUEUE_REMAPPING[queueName] : queueName
const queue = get(TASK_QUEUE_REMAPPING, queueName, queueName)

@SavelevMatthew SavelevMatthew force-pushed the feat/condo/INFRA-300/send-message-priority branch from f154da5 to 5fea60a Compare April 11, 2024 11:00
Copy link

sonarcloud bot commented Apr 11, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@SavelevMatthew SavelevMatthew merged commit ead428e into master Apr 11, 2024
23 checks passed
@SavelevMatthew SavelevMatthew deleted the feat/condo/INFRA-300/send-message-priority branch April 11, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😎 Cool ✋🙂 Review please Comments are resolved, take a look, please 👶 small Easy to review changes up to 50 lines of code
Development

Successfully merging this pull request may close these issues.

None yet

3 participants