Transfer control for the migration #9997
Conversation
WalkthroughThe migration workflow was updated to include a conditional check that ensures the migration transfer only proceeds if the source has no errors. If errors are present in the source, the transfer step is skipped, but shutdown procedures for both source and destination still execute as before. Changes
Sequence Diagram(s)sequenceDiagram
participant Worker
participant Source
participant Destination
Worker->>Source: getErrors()
alt No errors
Worker->>Worker: Set stage to "migrating"
Worker->>Worker: Update migration document
Worker->>Destination: Execute transfer
else Source has errors
Worker->>Worker: Skip transfer run
end
Worker->>Destination: Shutdown
Worker->>Source: Shutdown
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Appwrite/Platform/Workers/Migrations.php (2)
282-296: Also updatestatuswhen switching to the migrating stageGreat call adding the pre-check that skips the transfer when the source already contains errors.
However, we now only updatestagetomigratingwhile leavingstatusat its previous value (processing).
Down-stream consumers may rely on both fields evolving in tandem – all other stage transitions in this worker update both attributes.- $migration->setAttribute('stage', 'migrating'); + $migration + ->setAttribute('stage', 'migrating') + ->setAttribute('status', 'migrating');This keeps the state machine self-consistent and avoids confusion in dashboards or client SDKs that watch the document.
288-292: Use exceptions on JSON encoding to surface silent failures
json_encode()can silently returnfalse(e.g. on UTF-8 errors).
LeveragingJSON_THROW_ON_ERRORmakes data-loss obvious and will bubble the problem up to existing error handling.- $migration->setAttribute('resourceData', json_encode($transfer->getCache())); - $migration->setAttribute('statusCounters', json_encode($transfer->getStatusCounters())); + $migration->setAttribute('resourceData', json_encode($transfer->getCache(), JSON_THROW_ON_ERROR)); + $migration->setAttribute('statusCounters', json_encode($transfer->getStatusCounters(), JSON_THROW_ON_ERROR));This minor change improves robustness with negligible overhead.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Appwrite/Platform/Workers/Migrations.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Appwrite/Platform/Workers/Migrations.php (2)
src/Appwrite/SDK/Method.php (1)
getErrors(291-294)app/realtime.php (1)
getCache(127-147)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist
Summary by CodeRabbit