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

LSIF: Replace node-resque with bull #6062

Merged
merged 4 commits into from Oct 17, 2019
Merged

LSIF: Replace node-resque with bull #6062

merged 4 commits into from Oct 17, 2019

Conversation

efritz
Copy link
Contributor

@efritz efritz commented Oct 16, 2019

This PR replaces our dependency on node-resque with bull. I made a bad initial choice that caused a bit more friction than necessary when splitting the server and worker.

The latter library is 12x as popular on npm and seems more active in general. This new library has additional features we'll want if we end up sticking with a redis-based queue for work (job progress reporting, priority queues, cron scheduling). This simplifies our setup code a great deal and has working type declarations we don't need to declare ourselves.

I'm doing this now as a scheduled enqueue is the easiest way for us to do work on a schedule (required for RFC 43) while ensuring that (1) doesn't interfere with queries or ongoing conversion, and (2) isn't done by multiple processes.

@codecov
Copy link

codecov bot commented Oct 16, 2019

Codecov Report

Merging #6062 into master will increase coverage by 0.03%.
The diff coverage is 3.84%.

@@            Coverage Diff             @@
##           master    #6062      +/-   ##
==========================================
+ Coverage   39.45%   39.48%   +0.03%     
==========================================
  Files        1167     1167              
  Lines       58041    57985      -56     
  Branches     5737     5732       -5     
==========================================
- Hits        22900    22898       -2     
+ Misses      33023    32968      -55     
- Partials     2118     2119       +1
Impacted Files Coverage Δ
lsif/src/worker.ts 0% <0%> (ø) ⬆️
lsif/src/server.ts 0% <0%> (ø) ⬆️
lsif/src/worker.metrics.ts 0% <0%> (ø) ⬆️
lsif/src/queue.ts 0% <0%> (ø) ⬆️
lsif/src/tracing.ts 52.17% <100%> (ø) ⬆️
...d/frontend/internal/authz/bitbucketserver/store.go 70.86% <0%> (-0.87%) ⬇️

@efritz efritz marked this pull request as ready for review October 16, 2019 16:41
@efritz efritz requested a review from a team as a code owner October 16, 2019 16:41
@efritz efritz added the lsif label Oct 16, 2019
@efritz efritz changed the title WIP LSIF: Replace node-resque with bull Oct 16, 2019
Copy link
Contributor

@chrismwendt chrismwendt left a comment

Choose a reason for hiding this comment

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

Code LGTM, I also tried it out locally and it works ✨

@efritz efritz merged commit f022e03 into master Oct 17, 2019
@efritz efritz deleted the better-resque branch October 17, 2019 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants