Add guide for migrating Sidekiq job queues to Standalone Activities - #5202
Add guide for migrating Sidekiq job queues to Standalone Activities#5202brianmacdonald-temporal wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📖 Docs PR preview links
|
There was a problem hiding this comment.
Pull request overview
Adds a Ruby migration guide for converting Sidekiq jobs into Temporal Standalone Activities.
Changes:
- Adds the Sidekiq migration tutorial.
- Adds the guide to sidebar navigation.
- Adds a Ruby migration card to the Guides grid.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
docs/guides/sidekiq-to-standalone-activity.mdx |
Adds the migration guide. |
sidebars.js |
Adds sidebar navigation. |
src/components/GuidesGrid/guides-data.json |
Adds the guide card. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
chris-olszewski
left a comment
chris-olszewski
left a comment
There was a problem hiding this comment.
Overall looks good to me. I do think keeping the sidekiq / activity definition as close as possible will help illustrate how straightforward this migration is.
Also passed this onto @GregoryTravis who implemented SAA in Ruby for additional 👀
| user = get_user(input["user_id"]) | ||
| deliver_email(user.email, "Welcome!") | ||
| "sent to #{user.email}" |
There was a problem hiding this comment.
I find it a little odd that the activity definition doesn't match the sidekiq job. It gives the appearance of SAA needing to be handled in a special manner. Maybe having shared get_user/deliver_email helpers between the sidekiq job and activity to show that they use the same exact logic.
|
|
||
| # --- Mock helpers ------------------------------------------------------- | ||
| # Replace these with your real user lookup and mailer when you adapt this. | ||
| def get_user(user_id) |
There was a problem hiding this comment.
I agree with what @chris-olszewski said above -- perhaps have a single class that incorporates the functions below as methods, and include it in both code samples, so the user could actually copy + paste the whole example and see it run.
| # my_activity.rb (excerpt) | ||
| require "temporalio/error" | ||
|
|
||
| def execute(input) |
There was a problem hiding this comment.
| def execute(input) | |
| # ... | |
| def execute(input) |
Indent the method so it matches the full source from before.
|
|
||
| def execute(input) | ||
| user = get_user(input["user_id"]) | ||
| raise Temporalio::Error::ApplicationError.new( |
There was a problem hiding this comment.
since get_user() (above) always returns a value, and since you say that the mock deliver_email will throw, I think you can leave this out.
Ran the guide end to end against Ruby 3.4.1, temporalio gem 1.7.0, and Temporal CLI 1.8.2 (Server 1.31.2), and corrected what did not hold up: - Step 9: the List Filter example used `Status`, which the server rejects with "column name 'status' is not a valid search attribute". The attribute is `ExecutionStatus`. - Step 6: reword the deduplication claim. The SDK defaults are ALLOW_DUPLICATE for Id reuse and FAIL for Id conflict, so reusing an Id after completion starts a second execution; only concurrent duplicates are rejected. - Step 7: drop the note about varying the Activity Id between steps, which followed from the overstated dedup claim. - Step 4: extract the mock user lookup and mailer into welcome_email_helpers.rb so the Sidekiq job and the Activity share the same business logic, and show the shared file in the project layout. - Frontmatter: replace the unused `keywords` field with `tags` matching the sibling Celery migration guide, and expand the meta description. - Prerequisites: require Ruby 3.3 or higher. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The visibility query uses the wrong status field, and the documentation structure and metadata need adjustment.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
docs/guides/sidekiq-to-standalone-activity.mdx:5
- This description largely repeats the title and is shorter than the repository's roughly 120–155 character target, so it wastes the search-result snippet instead of adding context. Use a complementary summary of the concrete migration outcomes.
description: Migrate Sidekiq background jobs to a Temporal Standalone Activity for reliability and durable execution.
docs/guides/sidekiq-to-standalone-activity.mdx:54
- The page has 13 top-level headings, exceeding the repository target of fewer than 10. Group related steps under a few task-oriented H2 sections (for example, setup, migration, and operation) and make the individual steps H3s so the page hierarchy reflects its phases.
## Step 1: Set up your project directory
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
What does this PR do?
Adds a Ruby guide, Migrate a Sidekiq job queue to a Temporal Standalone Activity, that walks through converting a Sidekiq job into a Standalone Activity:
sidekiqprocess, Redis,perform_async,sidekiq_options retry:, Web UI) onto their Temporal equivalentsperform_asyncwithclient.start_activity(), and adds a return value viaclient.execute_activity()— something Sidekiq's fire-and-forget model can't providesidekiq_options retry: Nto a Retry Policy (with the total-attempts vs. retries adjustment:retry: 5->max_attempts: 6)client.list_activities()andclient.count_activities()in place of the Sidekiq Web UIAlso links the page from the Guides sidebar and adds a card to the Guides landing-page grid under the existing
Migrationtag.Notes to reviewers
Opened as a draft: the guide documents Standalone Activities, which are
publicPreviewinsrc/constants/featureReleaseTypes.js.All Temporal doc links are relative to satisfy
Temporal.RelativeLinks. Sidekiq-specific external links (sidekiq.org) remain absolute.Two review conventions from #5135 are already applied: core identifiers use
Id(Activity Id, Run Id) with generic "user identifier", and product name is "Temporal Service".