feat: show expires at and remaining columns in lease listing#343
feat: show expires at and remaining columns in lease listing#343raballew wants to merge 2 commits intojumpstarter-dev:mainfrom
Conversation
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change updates lease listing output to show "EXPIRES AT" and "REMAINING" instead of "BEGIN TIME" and "DURATION", implements helper methods on Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
e2e/tests.bats (1)
345-357: Make this E2E test self-contained to avoid order dependence.This test depends on suite-prepared state (
test-client-oidc, active exporters) and usesjmp delete leases --all, which broadens side effects beyond the lease created here. Please make setup/cleanup local to this test so it can run deterministically in isolation.Based on learnings: "E2E tests in bats should be runnable in isolation. Do not rely on prior suite setup or shared state across tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/tests.bats` around lines 345 - 357, This test is order-dependent because it relies on pre-existing client and exporter state and uses a global cleanup; modify the test to be self-contained by creating and using a unique test client (instead of relying on "jmp config client use test-client-oidc"), ensure any required exporter is started or validated with wait_for_exporter, create the lease with a unique selector/label so it’s identifiable, capture the created lease identifier from the "jmp create lease" output, assert on "jmp get leases" as before, and then delete only that specific lease (and the test client) rather than calling "jmp delete leases --all" so setup and teardown are local to the test.python/packages/jumpstarter/jumpstarter/client/grpc_test.py (1)
443-449: Add a deterministic non-expiredREMAININGassertion.Current tests validate
expiredandNone, but not the positive formatting output (Xd Yh Zm). Adding one fixed-clock test would better protect the new display behavior from regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter/jumpstarter/client/grpc_test.py` around lines 443 - 449, Add a deterministic positive-case test for Lease._format_remaining that asserts a future datetime produces the expected "Xd Yh Zm" string: create a fixed "now" (e.g., datetime(2020,1,1,0,0,0)), compute a future time (e.g., now + timedelta(days=1, hours=2, minutes=3)), call Lease._format_remaining(future_time) and assert it equals "1d 2h 3m"; add this as test_format_remaining_future in grpc_test.py and ensure the test uses the fixed clock (or computes expected output from that fixed reference) so it is deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e/tests.bats`:
- Around line 345-357: This test is order-dependent because it relies on
pre-existing client and exporter state and uses a global cleanup; modify the
test to be self-contained by creating and using a unique test client (instead of
relying on "jmp config client use test-client-oidc"), ensure any required
exporter is started or validated with wait_for_exporter, create the lease with a
unique selector/label so it’s identifiable, capture the created lease identifier
from the "jmp create lease" output, assert on "jmp get leases" as before, and
then delete only that specific lease (and the test client) rather than calling
"jmp delete leases --all" so setup and teardown are local to the test.
In `@python/packages/jumpstarter/jumpstarter/client/grpc_test.py`:
- Around line 443-449: Add a deterministic positive-case test for
Lease._format_remaining that asserts a future datetime produces the expected "Xd
Yh Zm" string: create a fixed "now" (e.g., datetime(2020,1,1,0,0,0)), compute a
future time (e.g., now + timedelta(days=1, hours=2, minutes=3)), call
Lease._format_remaining(future_time) and assert it equals "1d 2h 3m"; add this
as test_format_remaining_future in grpc_test.py and ensure the test uses the
fixed clock (or computes expected output from that fixed reference) so it is
deterministic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13b28fb4-15ef-4f17-9102-6798826273fe
📒 Files selected for processing (3)
e2e/tests.batspython/packages/jumpstarter/jumpstarter/client/grpc.pypython/packages/jumpstarter/jumpstarter/client/grpc_test.py
…rter-dev#32) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f958df9 to
6c0aafe
Compare
…eases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
212a314 to
583f539
Compare
Summary
BEGIN TIMEandDURATIONcolumns withEXPIRES ATandREMAININGinjmp get leasesoutputEXPIRES ATshows the calculated expiration timestampREMAININGshows human-readable time left (e.g.2h 15m) orexpiredFixes #32
Test plan
test_rich_add_columns_has_expires_at_and_remaining)test_compute_expires_at_*)test_format_remaining_*)test_rich_add_rows_*)jmp get leasesoutput contains EXPIRES AT and REMAINING columns🤖 Generated with Claude Code