Added timing to the provision command. #1850
Conversation
WalkthroughThe provisioning script was updated to track and log the total elapsed time of the site provisioning process. It records the start time at the beginning and, upon completion or early exit, calculates and displays the duration in minutes and seconds alongside the completion message. Corresponding tests were adjusted to match the updated finish message format by removing a trailing period. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
scripts/vortex/provision.sh(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
| start_time=$(date +%s) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Leverage built-in SECONDS or an EXIT trap for simpler, fail-safe timing
Storing start_time=$(date +%s) works, but Bash already exposes the SECONDS counter (seconds since shell start). Using it, or a one-liner trap, eliminates an external date call and guarantees the duration is printed even on unexpected exits:
- start_time=$(date +%s)
+trap 'duration=$SECONDS; info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)."' EXITBenefits:
• No subshell; one less external dependency
• Timing is emitted for every exit path without duplicating code
• Removes the risk of missing the finish log on future early-return branches
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| start_time=$(date +%s) | |
| trap 'duration=$SECONDS; info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)."' EXIT |
🤖 Prompt for AI Agents
In scripts/vortex/provision.sh around lines 81 to 82, replace the manual
start_time assignment using date with the built-in Bash SECONDS variable to
track elapsed time. Remove the external date call and instead use SECONDS to
measure duration. Additionally, add an EXIT trap function that prints the
elapsed time on script exit to ensure timing is logged regardless of how the
script terminates, avoiding duplicated timing code and improving reliability.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1850 +/- ##
===========================================
- Coverage 75.11% 75.11% -0.01%
===========================================
Files 84 84
Lines 4835 4838 +3
Branches 35 35
===========================================
+ Hits 3632 3634 +2
- Misses 1203 1204 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
force-pushed
the
feature/add-timing-provision
branch
from
4a78db5 to
68e20f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
scripts/vortex/provision.sh (1)
83-87: Early-return path omits elapsed-time informationThe “skip provisioning” branch still prints the old plain message (and with a trailing period) while the other paths show the timed variant. This breaks log consistency and may mislead tooling that parses the new format.
Either:
- Move the common “print duration” logic to an
EXITtrap (see previous comment), or- Call a shared
print_durationhelper here beforeexit 0.- info "Finished site provisioning." + print_duration # unified helper + exit 0
♻️ Duplicate comments (1)
scripts/vortex/provision.sh (1)
81-82: PreferSECONDS+ singleEXITtrap over a manualstart_timevariableUsing an external
date +%shere means:
• an extra subshell every time the duration is computed,
• duplicated bookkeeping further down the script,
• the risk that a futureexitpath forgets to compute the duration.Bash already exposes the running timer via
$SECONDS, and a one-liner trap guarantees the message is printed on all exits:-# Record start time -start_time=$(date +%s) + +# Emit provisioning duration on any exit +trap 'info "Finished site provisioning ($((SECONDS/60))m $((SECONDS%60))s)."' EXITThis removes two later blocks (lines 245-247 & 330-331) and the early-return inconsistency at lines 83-87.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.vortex/tests/bats/unit/provision.bats(8 hunks)scripts/vortex/provision.sh(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
scripts/vortex/provision.sh (2)
Learnt from: AlexSkrypnyk
PR: #1714
File: scripts/vortex/doctor.sh:69-70
Timestamp: 2025-06-03T03:15:29.849Z
Learning: In the Vortex project, logging functions (task(), info(), note(), pass(), fail(), warn()) are intentionally duplicated across all script files to make each script self-contained and independent, rather than sharing them through a common library. This design choice prioritizes script independence over reducing code duplication.
Learnt from: AlexSkrypnyk
PR: #1714
File: scripts/vortex/doctor.sh:69-70
Timestamp: 2025-06-03T03:15:29.849Z
Learning: In the Vortex project, logging functions (task(), info(), note(), pass(), fail(), warn()) are intentionally duplicated across all script files to make each script self-contained and independent, rather than sharing them through a common library. This design choice prioritizes script independence over reducing code duplication.
.vortex/tests/bats/unit/provision.bats (2)
Learnt from: AlexSkrypnyk
PR: drevops/vortex#0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in .vortex/installer/tests/Fixtures/install directory as they are test fixtures.
Learnt from: AlexSkrypnyk
PR: #1697
File: .vortex/installer/tests/Fixtures/install/provision_database_lagoon/docs/onboarding.md:16-17
Timestamp: 2025-06-02T00:41:36.321Z
Learning: DO NOT EVER REVIEW FILES IN .vortex/installer/tests/Fixtures/ directory - these are test fixtures and should not be reviewed under any circumstances.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
| "Finished site provisioning" | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Test expectations no longer cover the new “(Xm Ys)” suffix
The assertions were trimmed to "Finished site provisioning" (without the period) but the production script now appends timing info like (0m 03s)..
While substring matching will still pass, we lose coverage that the timing text is present and correctly formatted.
Consider tightening the check, e.g.:
assert_output_contains_regex 'Finished site provisioning \([0-9]\+m [0-9]\+s\)\.'(or equivalent helper) so the tests fail if the timing output is accidentally removed in the future.
Also applies to: 298-299, 438-439, 583-584, 723-724, 857-858, 998-999, 1117-1118
🤖 Prompt for AI Agents
In .vortex/tests/bats/unit/provision.bats around lines 167 to 168, the test
assertion only checks for the substring "Finished site provisioning" but does
not verify the appended timing suffix like "(Xm Ys).". Update the assertion to
use a regex that matches the full string including the timing format, for
example using assert_output_contains_regex with a pattern like 'Finished site
provisioning \([0-9]\+m [0-9]\+s\)\.'. Apply similar regex-based assertions to
the other specified line ranges to ensure the timing output is validated in all
relevant tests.
| duration=$(($(date +%s) - start_time)) | ||
| info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)." | ||
| exit 0 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Three copies of the same duration maths – extract once
The identical block
duration=$(($(date +%s) - start_time))
info "Finished site provisioning ($((duration / 60))m $((duration % 60))s)."appears twice (lines 245-247 & 330-331) and would need a third copy for the early-return path. Duplicate snippets are error-prone and already violate DRY.
If you don’t adopt the trap solution, at least factor this into a small helper:
print_duration() {
local duration=$(( $(date +%s) - start_time ))
info "Finished site provisioning ($((duration/60))m $((duration%60))s)."
}and call print_duration in all three places.
Also applies to: 330-331
🤖 Prompt for AI Agents
In scripts/vortex/provision.sh around lines 245-247 and 330-331, the duration
calculation and logging code is duplicated. To fix this, extract the repeated
code into a helper function named print_duration that calculates the duration
and logs the message. Then replace all occurrences of the duplicated code with
calls to this new function, including the early-return path, to adhere to DRY
principles and reduce error-prone duplication.
Summary by CodeRabbit