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

Update advice on queue config and running imports #65

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

asatwal
Copy link
Collaborator

@asatwal asatwal commented Feb 23, 2023

Trello-887

Add following advice to README.md:

  • Don't run a backfill when you've got a lot of traffic
  • Make sure you have a dedicated worker for BQ

@asatwal asatwal requested a review from duncanjbrown February 23, 2023 16:33
@asatwal asatwal self-assigned this Feb 23, 2023
@@ -12,7 +12,7 @@ en:
default: true
queue:
description: Which ActiveJob queue to put events on
default: ":default"
default: ":dfe_analytics"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is now at odds with

config.queue ||= :default

Copy link
Contributor

@duncanjbrown duncanjbrown left a comment

Choose a reason for hiding this comment

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

Good idea, thanks — imo the default behaviour is fine because it will work out of the box on Sidekiq? Fully support adding to the README tho

@asatwal asatwal force-pushed the add-more-advice-to-readme branch from 42cb68d to dd31c2a Compare February 24, 2023 10:58
@asatwal
Copy link
Collaborator Author

asatwal commented Feb 24, 2023

Good idea, thanks — imo the default behaviour is fine because it will work out of the box on Sidekiq? Fully support adding to the README tho

Yes, good point to have it working OOB. Restored actual config and left advice in the README.

@asatwal asatwal merged commit 6b20016 into main Feb 24, 2023
@asatwal asatwal deleted the add-more-advice-to-readme branch February 24, 2023 11:04
@asatwal asatwal mentioned this pull request Feb 24, 2023
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