Skip to content

feat: show expires at and remaining columns in lease listing#343

Open
raballew wants to merge 2 commits intojumpstarter-dev:mainfrom
raballew:022-lease-expiration-display
Open

feat: show expires at and remaining columns in lease listing#343
raballew wants to merge 2 commits intojumpstarter-dev:mainfrom
raballew:022-lease-expiration-display

Conversation

@raballew
Copy link
Copy Markdown
Contributor

Summary

  • Replace BEGIN TIME and DURATION columns with EXPIRES AT and REMAINING in jmp get leases output
  • EXPIRES AT shows the calculated expiration timestamp
  • REMAINING shows human-readable time left (e.g. 2h 15m) or expired

Fixes #32

Test plan

  • Unit tests: column headers include EXPIRES AT and REMAINING (test_rich_add_columns_has_expires_at_and_remaining)
  • Unit tests: expiration computed from effective_end_time, effective_begin_time+duration, begin_time+duration (test_compute_expires_at_*)
  • Unit tests: remaining time formatting including expired state (test_format_remaining_*)
  • Unit tests: table rendering with and without timing data (test_rich_add_rows_*)
  • E2E BATS test: verify jmp get leases output contains EXPIRES AT and REMAINING columns

🤖 Generated with Claude Code

@netlify
Copy link
Copy Markdown

netlify bot commented Mar 19, 2026

Deploy Preview for jumpstarter-docs ready!

Name Link
🔨 Latest commit 583f539
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/69bd69c0021adb00084db601
😎 Deploy Preview https://deploy-preview-343--jumpstarter-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 19, 2026

Warning

Rate limit exceeded

@raballew has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 27 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 47c8a00a-34ce-45ac-8a2d-fb037d6afd5d

📥 Commits

Reviewing files that changed from the base of the PR and between 6c0aafe and 583f539.

📒 Files selected for processing (1)
  • python/packages/jumpstarter-cli/jumpstarter_cli/get_test.py
📝 Walkthrough

Walkthrough

This change updates lease listing output to show "EXPIRES AT" and "REMAINING" instead of "BEGIN TIME" and "DURATION", implements helper methods on Lease to compute expiration and remaining time, and adds unit and end-to-end tests verifying the new display and computation logic.

Changes

Cohort / File(s) Summary
E2E Test
e2e/tests.bats
Added a Bats test that waits for exporter readiness, switches to test-client-oidc, creates a 1-day lease filtered by example.com/board=oidc, runs jmp get leases, asserts EXPIRES AT and REMAINING columns appear, then deletes leases.
Lease Display Logic
python/packages/jumpstarter/jumpstarter/client/grpc.py
Replaced BEGIN TIME/DURATION columns with EXPIRES AT/REMAINING. Added Lease._compute_expires_at() to determine expiration (prefers effective_end_time, else computes from begin + duration variants) and Lease._format_remaining() to render remaining time or "expired"/"". Refactored row rendering to use these helpers.
Unit Tests
python/packages/jumpstarter/jumpstarter/client/grpc_test.py
Added TestLeaseRichDisplay to assert column list exactly includes EXPIRES AT and REMAINING, covers _compute_expires_at() across timing scenarios (effective end, effective begin+duration, begin+duration, none), validates _format_remaining() for past/none, and checks rich table rendering with and without timing data.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • mangelajo
  • kirkbrauer

Poem

🐇 I nibbled clocks and hopped through time,
I found the leases’ end and made it shine.
"EXPIRES AT" gleams and "REMAINING" sings,
Now every lease knows when the bell rings.
Hooray — a hop, a fix, and tiny springs! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: replacing BEGIN TIME and DURATION columns with EXPIRES AT and REMAINING in the lease listing output.
Description check ✅ Passed The description clearly relates to the changeset, outlining the replacement of columns and providing a comprehensive test plan covering unit and E2E tests.
Linked Issues check ✅ Passed The changes fully implement the requirements from issue #32: replacing default lease listing columns with expiration time and remaining duration to show when a lease ends.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue #32 objective: updating lease column display, adding expiration computation and formatting logic, and comprehensive test coverage.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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 uses jmp 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-expired REMAINING assertion.

Current tests validate expired and None, 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

📥 Commits

Reviewing files that changed from the base of the PR and between bb7493b and f958df9.

📒 Files selected for processing (3)
  • e2e/tests.bats
  • python/packages/jumpstarter/jumpstarter/client/grpc.py
  • python/packages/jumpstarter/jumpstarter/client/grpc_test.py

@raballew raballew force-pushed the 022-lease-expiration-display branch from f958df9 to 6c0aafe Compare March 20, 2026 15:32
…eases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@raballew raballew force-pushed the 022-lease-expiration-display branch 2 times, most recently from 212a314 to 583f539 Compare March 20, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show lease expiration time by default

1 participant