[OSDEV-1388] Own waiter instead waiter from boto3#390
Conversation
React App | Jest test suite - Code coverage reportTotal: 27.8%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
doc/release/RELEASE-NOTES.md (1)
35-36: Improve formatting and remove duplicate entryThe bug fix entry about replacing the boto3 waiter appears to be a duplicate of the PR objective. Additionally, the formatting is inconsistent with other entries in the file.
Apply this diff to improve the formatting:
-* [OSDEV-1388](https://opensupplyhub.atlassian.net/browse/OSDEV-1388) - * The waiter from boto3 cannot wait more than half an hour so we replaced it with our own. +* [OSDEV-1388](https://opensupplyhub.atlassian.net/browse/OSDEV-1388) - The waiter from boto3 cannot wait more than half an hour so we replaced it with our own.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- deployment/terraform/database_anonymizer_sheduled_task/docker/database_anonymizer.py (2 hunks)
- doc/release/RELEASE-NOTES.md (1 hunks)
🔇 Additional comments (3)
deployment/terraform/database_anonymizer_sheduled_task/docker/database_anonymizer.py (2)
5-5: LGTM: Import addition is appropriateThe
timemodule import is correctly placed and necessary for the new polling implementation.
116-128: Verify timeout configuration across snapshot operationsThe script uses different timeout configurations for various operations:
- DB instance waiter: 120 attempts * 120s = 4 hours
- DB snapshot waiter: 100 attempts * 15s = 25 minutes
- New polling mechanism: 300 attempts * 60s = 5 hours
Please verify if these varying timeouts align with your requirements.
✅ Verification successful
Based on the search results, I can now generate a final response as I have sufficient context about the waiter configurations and their purpose from both the code and release notes.
Varying timeout configurations are intentionally designed and documented
The different timeout configurations across snapshot operations are intentionally designed to handle varying AWS RDS operation durations:
- DB instance waiter (4 hours): Used for database restoration, which can be a lengthy operation
- DB snapshot waiter (25 minutes): Used for standard snapshot completion
- Custom polling (5 hours): Specifically implemented to handle snapshot sharing, as noted in the release notes: "The waiter from boto3 cannot wait more than half an hour so we replaced it with our own"
The varying timeouts are not an inconsistency but rather a deliberate design choice to accommodate different AWS RDS operations and their typical completion times. The custom polling mechanism was specifically added to overcome boto3's waiter limitations for longer operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all waiter configurations in the codebase to ensure consistency rg -A 5 "waiter|time.sleep"Length of output: 3533
doc/release/RELEASE-NOTES.md (1)
Line range hint
1-1000: LGTM!The release notes are well-structured and provide clear documentation of changes across versions. The format is consistent, and the information is properly organized into relevant sections.
deployment/terraform/database_anonymizer_sheduled_task/docker/database_anonymizer.py
Show resolved
Hide resolved
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 56.14%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.91%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Django App | Unittest test suite - Code coverage reportTotal: 79.16%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
doc/release/RELEASE-NOTES.md (3)
10-10: Update the release dateThe release date is set to a future date (December 14, 2024). Please update it to the actual release date.
14-14: Fill in the placeholder sectionsSeveral sections contain placeholder text that needs to be replaced with actual content:
- Migrations
- Scheme changes
- Code/API changes
- Architecture/Environment changes
- What's new
- Release instructions
Please update these sections with the relevant changes being made in this release.
Also applies to: 17-17, 20-20, 23-23, 29-29, 32-32
12-12: Remove trailing colons from section headersThe section headers should not end with colons according to the markdown style guide.
Apply this change to the following headers:
- Database changes
- Migrations
- Scheme changes
- Code/API changes
- What's new
- Release instructions
Also applies to: 16-16, 19-19, 22-22, 28-28, 31-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
doc/release/RELEASE-NOTES.md(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
doc/release/RELEASE-NOTES.md
13-13: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
31-31: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)



The waiter from boto3 cannot wait more than half an hour so we replaced it with our own.