Skip to content

Conversation

@jclusso
Copy link
Member

@jclusso jclusso commented Jun 11, 2025

  • This works by overriding the perform_async method. I was surprised this works, but it does.
  • Additionally, Fabric::WebhookWorker has been removed. Using it is not ideal as it silences errors that Stripe would normally retry.

@jclusso jclusso requested a review from Copilot June 11, 2025 05:24

This comment was marked as outdated.

@jclusso jclusso requested a review from danarnold June 11, 2025 05:27
@jclusso jclusso force-pushed the add-configurable-worker-queue branch from 1d29fa2 to 6fd5cf2 Compare June 11, 2025 14:12
@jclusso jclusso requested a review from Copilot June 11, 2025 14:12

This comment was marked as outdated.

@jclusso
Copy link
Member Author

jclusso commented Jun 11, 2025

@danarnold I fixed this PR to support those options.

…orker

- This works by overriding the `perform_async` method. I was surprised this works, but it does.
- Additionally, Fabric::WebhookWorker has been removed. Using it is not ideal as it silences errors that Stripe would normally retry.
@jclusso jclusso force-pushed the add-configurable-worker-queue branch from 6fd5cf2 to 7206134 Compare June 11, 2025 17:42
@jclusso jclusso changed the title Add ability to configure a worker_queue Add ability to configure a worker_queue and remove Fabric::WebhookWorker Jun 11, 2025
@jclusso jclusso requested a review from Copilot June 11, 2025 17:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds the ability to configure the worker queue via a new Fabric.config attribute and removes the legacy Fabric::WebhookWorker to ensure errors aren’t silenced during Stripe webhook processing.

  • Overrides perform_async, perform_in, and perform_at in Worker to dynamically set the queue from configuration.
  • Removes the Fabric::WebhookWorker and updates the documentation and configuration accordingly.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
lib/fabric/app/workers/worker.rb Overrides asynchronous methods to use the configurable queue, removing the hard-coded 'fabric' queue.
lib/fabric/app/workers/webhook_worker.rb Completely removed as part of deprecating error-silencing webhook processing.
lib/fabric.rb Updates require statements and adds a new worker_queue configuration with default value 'critical'.
README.md Removes outdated asynchronous usage instructions referencing the removed WebhookWorker.
Comments suppressed due to low confidence (2)

lib/fabric/app/workers/worker.rb:38

  • Consider adding a brief inline comment explaining why perform_async, perform_in, and perform_at are overridden to set the queue dynamically using Fabric.config.worker_queue. This will improve maintainability and ease future modifications.
def perform_async(...)

lib/fabric.rb:145

  • It would be beneficial to include a comment or update related documentation to explain the purpose and expected values for the new worker_queue configuration attribute.
attr_accessor :worker_queue

@jclusso jclusso merged commit 777051f into 2024-04-10 Jun 13, 2025
2 checks passed
@jclusso jclusso deleted the add-configurable-worker-queue branch June 13, 2025 16:29
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.

3 participants