add online/offline indicator to the CLI output of jmp get exporters #584
Conversation
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds an optional online field to Exporter proto. Introduces options-driven display/serialization for exporters (online, leases) in gRPC client and config. Updates CLI to parse comma-separated flags for including fields. Adds common CLI parsing utilities. Expands tests for options parsing and rich table rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (get)
participant Common as CLI Common (opt)
participant Config as ClientConfigV1Alpha1
participant GRPC as gRPC Client
participant Proto as Server/Proto
User->>CLI: jumpstarter get exporters --with online,leases
CLI->>Common: parse_comma_separated("--with")
Common-->>CLI: ["online","leases"]
CLI->>Config: list_exporters(include_online=True, include_leases=True)
Config->>Proto: Fetch exporters (+leases, online)
Proto-->>Config: ExporterList (proto)
Config->>GRPC: Build Exporter objects (set online, lease)
GRPC-->>Config: ExporterList(include_online, include_leases)
Config-->>CLI: ExporterList
CLI->>GRPC: Render table via options
GRPC-->>CLI: Table with columns/rows
CLI-->>User: Display results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
changed to use "--with online" option. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py (2)
29-29: Remove trailing whitespace.Line 29 has trailing whitespace that needs to be removed.
- name="exporter-2", + name="exporter-2",
34-34: Remove whitespace from blank lines.Several blank lines contain whitespace characters that should be removed.
- +Also applies to: 43-43, 54-54
packages/jumpstarter/jumpstarter/client/grpc_test.py (1)
357-358: Remove trailing blank lines.The file ends with extra blank lines that should be removed for consistency.
- +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py(1 hunks)packages/jumpstarter/jumpstarter/client/grpc_test.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/jumpstarter/jumpstarter/client/grpc_test.py (1)
packages/jumpstarter/jumpstarter/client/grpc.py (5)
ExporterList(195-243)WithOptions(20-22)add_display_columns(25-35)add_exporter_row(38-53)get_status(178-192)
🪛 GitHub Check: ruff
packages/jumpstarter-cli/jumpstarter_cli/get_test.py
[failure] 54-54: Ruff (W293)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:54:1: W293 Blank line contains whitespace
[failure] 43-43: Ruff (W293)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:43:1: W293 Blank line contains whitespace
[failure] 34-34: Ruff (W293)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:34:1: W293 Blank line contains whitespace
[failure] 29-29: Ruff (W291)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:29:35: W291 Trailing whitespace
[failure] 9-9: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:9:18: F401 .get.get_exporters imported but unused
[failure] 4-4: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:4:27: F401 click.testing.CliRunner imported but unused
[failure] 3-3: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:3:8: F401 pytest imported but unused
[failure] 1-1: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:1:44: F401 unittest.mock.patch imported but unused
[failure] 1-1: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:1:27: F401 unittest.mock.AsyncMock imported but unused
[failure] 1-12: Ruff (I001)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:1:1: I001 Import block is un-sorted or un-formatted
🪛 GitHub Actions: Lint
packages/jumpstarter-cli/jumpstarter_cli/get_test.py
[error] 1-1: Ruff: Import block is un-sorted or un-formatted
⏰ 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). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (11)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py (4)
51-100: Test coverage for with_options parsing is comprehensive.The test methods thoroughly cover all scenarios for parsing
with_options:
- Individual options (
"leases","online")- Combined options
- Empty options
- Unknown options being ignored
The logic correctly verifies that only valid options are processed while invalid ones are ignored.
18-49: Good test helper design with flexible options.The
create_test_exportersmethod provides a clean way to generate test data with configurable lease and online status options. The mock setup for lease objects is appropriate and covers the necessary attributes.
101-118: ExporterList creation tests validate key attributes.The tests effectively verify that
ExporterListobjects are created with the correct configuration:
- Basic creation without options
- Creation with both
include_leasesandinclude_onlineflags- Proper validation of list length and option flags
120-152: Integration test provides good end-to-end validation.The integration test effectively validates the data flow from individual
Exporterobjects toExporterList, ensuring that:
- Exporters maintain their properties (name, online status, labels)
- The list correctly aggregates multiple exporters
- Display options are properly set and maintained
packages/jumpstarter/jumpstarter/client/grpc_test.py (7)
20-30: WithOptions tests provide solid baseline coverage.The tests correctly validate both default and custom configurations of the
WithOptionsdataclass, ensuring the boolean flags work as expected for controlling display features.
32-63: Comprehensive column addition testing.The
TestAddDisplayColumnsclass thoroughly tests all column combinations:
- Basic columns (NAME, LABELS)
- With online status column
- With lease columns (LEASED BY, LEASE STATUS, START TIME)
- All columns combined
The test assertions correctly verify the column headers in the expected order.
65-122: Row addition tests cover all scenarios effectively.The
TestAddExporterRowclass validates:
- Basic row addition
- Row with lease information
- Row with "Available" status for unleased exporters
- All options enabled with different online/lease states
The helper method
create_test_exporterprovides good flexibility for test scenarios.
124-194: ExporterList display tests validate core functionality.These tests effectively verify:
- Exporters display correctly without lease information
- Lease information is only shown when the display option is enabled
- Actual table rendering produces expected output content
The use of
StringIOandConsoleto test actual rendered output is a good approach for validating the end-user experience.
223-261: Online status display testing is thorough.The test validates both online and offline status indicators (✅ and ❌) appear correctly in the rendered output. The approach of testing actual rendered content ensures the icons are properly displayed.
262-309: Comprehensive integration test covers all features.This test effectively validates the complete feature set working together:
- Online/offline indicators
- Lease information for leased exporters
- "Available" status for unleased exporters
- Multiple exporters with different states
- All display options enabled simultaneously
The assertions verify all expected content appears in the rendered output.
310-356: Lease information extraction logic is well-tested.These tests validate the critical logic for extracting lease information:
- Correct extraction from lease objects (client, status, start time)
- Proper fallback to "Available" when no lease exists
- Direct testing of the tuple construction logic used in display
The manual verification approach provides confidence in the data extraction process.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/jumpstarter/jumpstarter/client/grpc.py (4)
19-23: WithOptions: small nit for clarityConsider adding a short docstring explaining that show_online/show_leases control table display (not data fetching), to distinguish them from include_online/include_leases on ExporterList.
35-36: Column naming nitpick“START TIME” could be more explicit as “LEASE START” (or “LEASED SINCE”) to reduce ambiguity in the table.
38-54: Deterministic labels output and ASCII fallback for online
- For stable CLI diffs, sort labels when joining.
- Some terminals don’t render emoji well; consider a text fallback (“ONLINE”/“OFFLINE” or “Yes”/“No”) or a flag to toggle emoji.
Apply this diff to sort labels deterministically:
- row_data.append(",".join(("{}={}".format(k, v) for k, v in exporter.labels.items()))) + row_data.append(",".join(f"{k}={v}" for k, v in sorted(exporter.labels.items())))
95-107: Row rendering is correct; timestamp format suggestionRendering lease status looks correct. Recommend ISO 8601 timestamps (with timezone/offset) for consistency with logs and JSON.
Apply this diff:
- if self.lease.effective_begin_time: - start_time = self.lease.effective_begin_time.strftime("%Y-%m-%d %H:%M:%S") + if self.lease.effective_begin_time: + start_time = self.lease.effective_begin_time.isoformat(timespec="seconds")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/jumpstarter/jumpstarter/client/grpc.py(4 hunks)packages/jumpstarter/jumpstarter/config/client.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jumpstarter/jumpstarter/config/client.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/jumpstarter/jumpstarter/client/grpc.py (1)
packages/jumpstarter/jumpstarter/client/lease.py (1)
Lease(31-211)
⏰ 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). (7)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
🔇 Additional comments (9)
packages/jumpstarter/jumpstarter/client/grpc.py (9)
25-36: Columns setup looks goodColumn composition matches the options and aligns with the PR goal. No functional issues spotted.
83-85: Exporter fields addition LGTMAdding online and an optional lease on Exporter is consistent with the new display behavior.
89-89: Proto compatibility: guard against older protocol versionsIf the client imports an older jumpstarter-protocol without the online field, accessing data.online raises AttributeError. To be resilient across mixed versions, default to False when absent.
Apply this diff:
- return cls(namespace=namespace, name=name, labels=data.labels, online=data.online) + return cls( + namespace=namespace, + name=name, + labels=data.labels, + online=getattr(data, "online", False), + )
92-94: rich_add_columns refactor LGTMDelegating to add_display_columns with options keeps concerns separated and readable.
198-199: ExporterList flags LGTMinclude_online/include_leases being excluded from serialization is appropriate.
208-216: ExporterList -> WithOptions wiring LGTMPassing options down ensures columns/rows stay in sync. Good guard against mismatched tables.
223-235: Lease JSON inclusion is inconsistent with intent; exclude lease when include_leases=FalseThe summary states leases are serialized separately only when include_leases=True. The else branch currently includes the lease field via exporter.model_dump(mode="json"), which may include a lease (even if None), diverging from that intent and changing the schema.
Apply this diff:
- else: - data = {"exporters": [exporter.model_dump(mode="json") for exporter in self.exporters]} + else: + data = {"exporters": [exporter.model_dump(mode="json", exclude={"lease"}) for exporter in self.exporters]}Likely an incorrect or invalid review comment.
238-250: Same fix for Python dict dumpMirror the JSON behavior so the Python object shape matches the intended contract when include_leases=False.
Apply this diff:
- else: - return {"exporters": [exporter.model_dump(mode="json") for exporter in self.exporters]} + else: + return {"exporters": [exporter.model_dump(mode="json", exclude={"lease"}) for exporter in self.exporters]}Likely an incorrect or invalid review comment.
92-96: Allrich_add_*call sites correctly handle options (default and explicit)I’ve verified that in the production code every invocation of
rich_add_columns/rich_add_rowseither passes an explicitWithOptionsor relies on theoptions=Nonedefault (which is covered by existing tests). No user‐facing code is inadvertently dropping the new parameter.
michalskrivanek
force-pushed
the
online
branch
2 times, most recently
from
d72181c to
2dd4f12
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py (1)
1-9: Clean up unused / unsorted imports
AsyncMock,patch,pytest,CliRunner, andget_exportersare still unused, and the import block remains unsorted, so Ruff keeps failing. Please drop the unused symbols and let an auto-formatter (e.g.ruff --fixorisort + black) reorder the remaining lines.
🧹 Nitpick comments (2)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py (2)
29-29: Strip trailing / stray whitespaceSeveral lines contain trailing spaces or blank lines with indentation, triggering Ruff
W291/W293. Removing them will make the linter green.Also applies to: 34-34, 43-43, 54-54
51-100: Reduce repetition with parametrised testsThe five almost-identical
with_optionsparsing tests differ only in input and expected booleans. Parametrising them withpytest.mark.parametrizewould cut ~40 lines and be easier to extend.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/jumpstarter-cli/jumpstarter_cli/get.py(1 hunks)packages/jumpstarter-cli/jumpstarter_cli/get_test.py(1 hunks)packages/jumpstarter/jumpstarter/client/grpc.py(4 hunks)packages/jumpstarter/jumpstarter/client/grpc_test.py(1 hunks)packages/jumpstarter/jumpstarter/config/client.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/jumpstarter-cli/jumpstarter_cli/get.py
- packages/jumpstarter/jumpstarter/config/client.py
- packages/jumpstarter/jumpstarter/client/grpc_test.py
- packages/jumpstarter/jumpstarter/client/grpc.py
🧰 Additional context used
🪛 GitHub Check: ruff
packages/jumpstarter-cli/jumpstarter_cli/get_test.py
[failure] 54-54: Ruff (W293)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:54:1: W293 Blank line contains whitespace
[failure] 43-43: Ruff (W293)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:43:1: W293 Blank line contains whitespace
[failure] 34-34: Ruff (W293)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:34:1: W293 Blank line contains whitespace
[failure] 29-29: Ruff (W291)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:29:35: W291 Trailing whitespace
[failure] 9-9: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:9:18: F401 .get.get_exporters imported but unused
[failure] 4-4: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:4:27: F401 click.testing.CliRunner imported but unused
[failure] 3-3: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:3:8: F401 pytest imported but unused
[failure] 1-1: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:1:44: F401 unittest.mock.patch imported but unused
[failure] 1-1: Ruff (F401)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:1:27: F401 unittest.mock.AsyncMock imported but unused
[failure] 1-12: Ruff (I001)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py:1:1: I001 Import block is un-sorted or un-formatted
🪛 GitHub Actions: Lint
packages/jumpstarter-cli/jumpstarter_cli/get_test.py
[error] 1-1: Ruff: Import block is un-sorted or un-formatted.
⏰ 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). (10)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (12)
packages/jumpstarter/jumpstarter/client/grpc_test.py (12)
30-60: Reduce duplication and add idempotency check for column addition
- Consider parametrizing these four cases to cut repetition and improve readability.
- Add a test that calling add_display_columns twice does not duplicate columns.
63-71: Add a test for empty/missing labelsCreate one exporter with labels={} and one with labels=None to verify graceful rendering (no crashes, sensible display like empty string).
73-83: Also assert rendered content, not only countsIn addition to row/column counts, render and assert presence of “test-exporter” and a label like “env=test”. This catches accidental regressions in display formatting.
84-107: Validate lease content in rendered outputCounts look good. Also assert that “client-123”, “Active”, and “2023-01-01 10:00:00” appear in the rendered table to ensure correct cell population.
122-127: Consider timezone-aware lease timestampsUsing naive datetimes can be ambiguous. Prefer timezone-aware datetimes (e.g., UTC) to avoid formatting inconsistencies.
Example:
-from datetime import datetime +from datetime import datetime, timezone ... -lease.effective_begin_time = datetime(2023, 1, 1, 10, 0, 0) +lease.effective_begin_time = datetime(2023, 1, 1, 10, 0, 0, tzinfo=timezone.utc)
144-161: Also assert absence of lease info in rendered output when show_leases=FalseRender and assert that “LEASED BY”, “LEASE STATUS”, and “START TIME” (or associated data) are not present to prevent accidental leakage.
181-184: Prefer Rich’s record mode over StringIO for capturing outputThis is more idiomatic and resilient across environments.
- console = Console(file=StringIO(), width=120) - console.print(table) - output = console.file.getvalue() + console = Console(record=True, width=120) + console.print(table) + output = console.export_text()
209-212: Use record mode capture here as wellApply the same capture approach for consistency and robustness.
- console = Console(file=StringIO(), width=120) - console.print(table) - output = console.file.getvalue() + console = Console(record=True, width=120) + console.print(table) + output = console.export_text()
249-252: Use record mode capture here as well- console = Console(file=StringIO(), width=120) - console.print(table) - output = console.file.getvalue() + console = Console(record=True, width=120) + console.print(table) + output = console.export_text()
291-294: Use record mode capture here as well- console = Console(file=StringIO(), width=150) - console.print(table) - output = console.file.getvalue() + console = Console(record=True, width=150) + console.print(table) + output = console.export_text()
253-258: Emoji assertions can be brittle across environmentsTerminals without emoji support may still run tests (CI containers, Windows code pages). Add fallbacks such as accepting “✓/✗” or textual “ONLINE/OFFLINE” indicators when emojis aren’t present.
333-353: Similarly, test the no-lease path via the shared helperUse the same helper to assert the default (“”, “Available”, “”) case instead of open-coding the logic here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/jumpstarter-cli/jumpstarter_cli/get.py(1 hunks)packages/jumpstarter-cli/jumpstarter_cli/get_test.py(1 hunks)packages/jumpstarter/jumpstarter/client/grpc.py(4 hunks)packages/jumpstarter/jumpstarter/client/grpc_test.py(1 hunks)packages/jumpstarter/jumpstarter/config/client.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/jumpstarter-cli/jumpstarter_cli/get.py
- packages/jumpstarter/jumpstarter/config/client.py
- packages/jumpstarter-cli/jumpstarter_cli/get_test.py
- packages/jumpstarter/jumpstarter/client/grpc.py
⏰ 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). (5)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
🔇 Additional comments (4)
packages/jumpstarter/jumpstarter/client/grpc_test.py (4)
18-27: LGTM: Default and custom WithOptions coverage is correctBoth defaults are False and custom True/True path is validated.
108-119: LGTM: Combined options row shape verifiedColumn shape for NAME, ONLINE, LABELS, LEASED BY, LEASE STATUS, START TIME is correct.
129-143: LGTM: Base exporter rendering without leasesEnsures default two-column view behaves as expected.
259-306: LGTM: End-to-end coverage for online + lease displayGood integration test validating columns and critical content across both rows.
| def test_exporter_lease_info_extraction(self): | ||
| """Test that lease information is correctly extracted from lease objects""" | ||
| lease = self.create_test_lease(client="my-client", status="Expired") | ||
| exporter = Exporter( | ||
| namespace="default", | ||
| name="test-exporter", | ||
| labels={"type": "device"}, | ||
| online=True, | ||
| lease=lease | ||
| ) | ||
|
|
||
| # Manually verify the lease data that would be extracted | ||
| assert exporter.lease.client == "my-client" | ||
| assert exporter.lease.get_status() == "Expired" | ||
| assert exporter.lease.effective_begin_time.strftime("%Y-%m-%d %H:%M:%S") == "2023-01-01 10:00:00" | ||
|
|
||
| # Test the logic that builds lease_info tuple in rich_add_rows | ||
| options = WithOptions(show_leases=True) | ||
| if options.show_leases and exporter.lease: | ||
| lease_client = exporter.lease.client | ||
| lease_status = exporter.lease.get_status() | ||
| start_time = exporter.lease.effective_begin_time.strftime("%Y-%m-%d %H:%M:%S") | ||
| lease_info = (lease_client, lease_status, start_time) | ||
|
|
||
| assert lease_info == ("my-client", "Expired", "2023-01-01 10:00:00") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid re-implementing production logic in tests; extract and test a helper
This test mirrors rich_add_rows’ lease-info construction. Prefer introducing a small helper in the production module and test that directly to reduce duplication and brittleness.
Example production helper (in jumpstarter.client.grpc):
def extract_lease_info(lease) -> tuple[str, str, str]:
if not lease:
return ("", "Available", "")
client = getattr(lease, "client", "")
status = lease.get_status() if hasattr(lease, "get_status") else ""
dt = getattr(lease, "effective_begin_time", None)
start = dt.strftime("%Y-%m-%d %H:%M:%S") if dt else ""
return (client, status, start)Then the test can just assert extract_lease_info(lease) == ("my-client", "Expired", "2023-01-01 10:00:00").
🤖 Prompt for AI Agents
In packages/jumpstarter/jumpstarter/client/grpc_test.py around lines 307 to 332,
the test duplicates the lease-info extraction logic found in production code. To
fix this, move the lease-info extraction logic into a small helper function in
the production module (jumpstarter.client.grpc), such as
extract_lease_info(lease), that returns the lease info tuple. Then update the
test to call this helper and assert its output instead of re-implementing the
logic, reducing duplication and brittleness.
kirkbrauer
left a comment
kirkbrauer
left a comment
There was a problem hiding this comment.
I think this looks pretty awesome! Good to see some of the refactoring around how we handle outputting tables/JSON/YAML. See comments.
| @opt_selector | ||
| @opt_output_all | ||
| @click.option("--with", "with_options", multiple=True, help="Include additional information (e.g., 'leases')") | ||
| @click.option("--with", "with_options", multiple=True, help="Include additional information ('leases', 'online')") |
There was a problem hiding this comment.
@michalskrivanek do we have any other references to other Cloud Native tools that use this format for including optional values in the output?
There was a problem hiding this comment.
I haven't looked TBH, just followed the original --with implementation here.
i guess we can argue about "--with x --with y" vs "--with x,y"....from a casual look the latter seem more common
There was a problem hiding this comment.
i feel like both deserve to be part of the default output
There was a problem hiding this comment.
@michalskrivanek do we have any other references to other Cloud Native tools that use this format for including optional values in the output?
I don't think so, but this is more related to the user-side of jumpstarter than the interaction with the cloud/cluster itself.
The problem is that we have way too much output on screen, and the terminal chokes.
i feel like both deserve to be part of the default output
I had the same feeling, but the output would be very busy. May be we could start here, and instead make the labels optional?
adding a new column reflecting the exporter state. Refactored the display functions to allow for dynamic columns so the --with options can be combined.
michalskrivanek
force-pushed
the
online
branch
from
2dd4f12 to
4105a7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/jumpstarter/jumpstarter/client/grpc.py (1)
38-45: Plain-text online indicator confirmed (yes/no).This aligns with prior guidance to avoid emojis. Nice.
packages/jumpstarter/jumpstarter/client/grpc_test.py (1)
307-353: Refactor test logic to avoid duplicating production lease-info extraction.This block re-implements the tuple-building logic from production. Prefer to import and test a small helper from the module (e.g., extract_lease_info) and assert its output.
Example test change:
- options = WithOptions(show_leases=True) - if options.show_leases and exporter.lease: - lease_client = exporter.lease.client - lease_status = exporter.lease.get_status() - start_time = exporter.lease.effective_begin_time.strftime("%Y-%m-%d %H:%M:%S") - lease_info = (lease_client, lease_status, start_time) - assert lease_info == ("my-client", "Expired", "2023-01-01 10:00:00") + from jumpstarter.client.grpc import extract_lease_info + assert extract_lease_info(exporter.lease) == ("my-client", "Expired", "2023-01-01 10:00:00")Do likewise for the “no lease” test.
🧹 Nitpick comments (9)
packages/jumpstarter/jumpstarter/client/grpc.py (3)
19-23: Bold move to centralize display toggles.The WithOptions dataclass is clear and keeps column concerns out of model code. Consider making it frozen to avoid accidental mutation across flows, but not a blocker.
25-36: Column builder is straightforward and option-driven.Minor nit: consider documenting the column order contract since downstream tests and consumers rely on it.
83-90: from_protobuf: online field presence and backward-compatibility.If talking to older controllers that don’t set the optional field, defaulting to False will render everything “no”. If that’s acceptable per the dependency PRs, fine; otherwise you can defensively read with getattr or gate display based on negotiated capability.
Minimal defensive tweak:
- return cls(namespace=namespace, name=name, labels=data.labels, online=data.online) + return cls( + namespace=namespace, + name=name, + labels=data.labels, + online=getattr(data, "online", False), + )packages/jumpstarter/jumpstarter/client/grpc_test.py (6)
37-44: Column set for online-only display is validated well.Good coverage on column order. You might also assert that no lease-related headers appear in this case.
84-95: Row rendering with explicit lease_info looks correct.Consider adding a small assertion on rendered output to verify the lease tuple values appear, mirroring later tests.
221-258: Docstring still mentions “icons” while output uses plain text yes/no.Update the docstring and assertion comments to say “indicator” to avoid confusion.
121-179: “TestExporterList” class name is misleading; these tests exercise Exporter display, not ExporterList.Either rename the class or add dedicated tests for ExporterList to verify:
- Options propagation (include_online/include_leases).
- model_dump and model_dump_json include/exclude lease correctly.
180-206: Nice end-to-end render assertions for lease display.Consider adding a negative assertion that lease client is absent when exporter has no lease and show_leases=True (you already assert “Available”).
259-306: Good combined coverage for online + lease columns.Once the serialization fixes land, consider adding a small test that ExporterList.model_dump_json excludes or includes lease data based on include_leases to prevent regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/jumpstarter-cli/jumpstarter_cli/get.py(1 hunks)packages/jumpstarter-cli/jumpstarter_cli/get_test.py(1 hunks)packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py(3 hunks)packages/jumpstarter/jumpstarter/client/grpc.py(4 hunks)packages/jumpstarter/jumpstarter/client/grpc_test.py(1 hunks)packages/jumpstarter/jumpstarter/config/client.py(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/jumpstarter-cli/jumpstarter_cli/get_test.py
- packages/jumpstarter-cli/jumpstarter_cli/get.py
- packages/jumpstarter/jumpstarter/config/client.py
- packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/jumpstarter/jumpstarter/client/grpc_test.py (2)
packages/jumpstarter/jumpstarter/client/grpc.py (4)
WithOptions(20-22)add_display_columns(25-35)add_exporter_row(38-53)get_status(178-192)packages/jumpstarter/jumpstarter/exporter/exporter.py (1)
status(115-134)
packages/jumpstarter/jumpstarter/client/grpc.py (3)
packages/jumpstarter/jumpstarter/common/metadata.py (1)
name(13-14)packages/jumpstarter/jumpstarter/client/lease.py (1)
Lease(31-211)packages/jumpstarter/jumpstarter/config/exporter.py (3)
rich_add_columns(190-192)rich_add_rows(194-199)rich_add_names(201-203)
⏰ 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). (7)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: e2e
🔇 Additional comments (1)
packages/jumpstarter/jumpstarter/client/grpc.py (1)
195-216: ExporterList’s include_online/include_leases flags and option plumbing look good.This keeps display concerns at the list level and avoids leaking flags into the model.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/jumpstarter-cli/jumpstarter_cli/get.py (1)
31-31: Type hint nit: prefer Sequence[str] for flexibilityClick callbacks can return list or tuple depending on configuration; using a more general type avoids churn.
-def get_exporters(config, selector: str | None, output: OutputType, with_options: list[str]): +from collections.abc import Sequence +def get_exporters(config, selector: str | None, output: OutputType, with_options: Sequence[str]):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/jumpstarter-cli/jumpstarter_cli/get.py(2 hunks)packages/jumpstarter-cli/jumpstarter_cli/get_test.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jumpstarter-cli/jumpstarter_cli/get_test.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/jumpstarter-cli/jumpstarter_cli/get.py (5)
packages/jumpstarter-cli-common/jumpstarter_cli_common/config.py (1)
callback(22-24)packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py (1)
handle_exceptions_with_reauthentication(49-74)packages/jumpstarter-cli/jumpstarter_cli/login.py (1)
relogin_client(141-155)packages/jumpstarter/jumpstarter/config/client.py (1)
list_exporters(144-183)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exporters.py (1)
list_exporters(126-131)
⏰ 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). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: e2e
🔇 Additional comments (1)
packages/jumpstarter-cli/jumpstarter_cli/get.py (1)
37-38: LGTM: includes online flag and forwards it correctlyThe include_online toggle and forwarding to config.list_exporters align with the new protobuf/model surface.
instead of multimple --with paramters we can do --with option1,option2
michalskrivanek
force-pushed
the
online
branch
from
af57716 to
11e281a
Compare
|
addad a commit refactoring the parsing into the common opt helper. Maybe too much of AI generated cruft...OTOH maybe the right thing to unify parsing in other places. Let me know if it's too much, it can be dropped |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py (4)
142-151: Simplify deduplication using dict.fromkeysFunctionally correct and order-preserving. You can simplify it for readability.
-def _deduplicate_tokens(tokens: list[str]) -> list[str]: - """Remove duplicates while preserving order.""" - seen = set() - unique_tokens = [] - for token in tokens: - if token not in seen: - seen.add(token) - unique_tokens.append(token) - return unique_tokens +def _deduplicate_tokens(tokens: list[str]) -> list[str]: + """Remove duplicates while preserving order.""" + return list(dict.fromkeys(tokens))
153-168: Surface all invalid values at once to improve UXCurrent validation raises on the first invalid token. Reporting all invalid values helps users fix inputs in one run.
def _validate_tokens( tokens: list[str], allowed_values: set[str], ctx: click.Context, param: click.Parameter ) -> None: """Validate tokens against allowed values.""" - for token in tokens: - if token not in allowed_values: - allowed_list = ", ".join(sorted(allowed_values)) - raise click.BadParameter( - f"Invalid value '{token}'. Allowed values are: {allowed_list}", - ctx=ctx, - param=param - ) + invalid = [t for t in tokens if t not in allowed_values] + if invalid: + allowed_list = ", ".join(sorted(allowed_values)) + invalid_list = ", ".join(invalid) + raise click.BadParameter( + f"Invalid value(s): {invalid_list}. Allowed values are: {allowed_list}", + ctx=ctx, + param=param, + )
170-211: Normalize allowed_values when normalize_case=True to avoid false negativesIf callers pass mixed-case allowed_values while normalize_case=True (default), valid inputs may be rejected. Normalize the allowed set to match input normalization.
# Validate if allowed values are specified - if allowed_values is not None: - _validate_tokens(unique_tokens, allowed_values, ctx, param) + if allowed_values is not None: + # Normalize allowed values to match input normalization behavior + if normalize_case: + allowed_values = {v.lower() for v in allowed_values} + _validate_tokens(unique_tokens, allowed_values, ctx, param)
213-248: Option factory: explicitly set type=str and refine auto help (case-insensitive hint)
- Setting type=str is more explicit and consistent with other options.
- Auto-generated help can mention case-insensitivity when normalize_case=True. This matches user expectations for flags like --with ONLINE.
# Auto-generate help text if not provided if help_text is None: if allowed_values: - allowed_list = ", ".join(sorted(allowed_values)) - help_text = f"Comma-separated values. Allowed: {allowed_list} (comma-separated or repeated)" + allowed_list = ", ".join(sorted(v.lower() if normalize_case else v for v in allowed_values)) + case_hint = " (case-insensitive)" if normalize_case else "" + help_text = f"Include fields: {allowed_list} (comma-separated or repeated){case_hint}" else: help_text = "Comma-separated values (comma-separated or repeated)" return click.option( f"--{name}", f"{name}_options", callback=callback, multiple=True, + type=str, help=help_text )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py(1 hunks)packages/jumpstarter-cli/jumpstarter_cli/get.py(2 hunks)packages/jumpstarter-cli/jumpstarter_cli/get_test.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/jumpstarter-cli/jumpstarter_cli/get_test.py
- packages/jumpstarter-cli/jumpstarter_cli/get.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py (2)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py (1)
allowed_values(15-17)packages/jumpstarter-cli-common/jumpstarter_cli_common/config.py (1)
callback(22-24)
⏰ 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). (10)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: e2e
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (1)
packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py (1)
129-139: Solid token normalization — LGTMGood handling of trimming, optional case normalization, and dropping empty tokens. This should handle both CSV and repeated flag inputs reliably.
it can be reused in other places now, with or without allowed values or case sensitivity
as part of exporters
michalskrivanek
force-pushed
the
online
branch
from
11e281a to
d74a937
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/jumpstarter/jumpstarter/client/grpc.py (3)
38-54: Plain “yes/no” for ONLINE is the right call hereThis addresses the earlier feedback about avoiding emojis and fits the separate ONLINE column well. Sorting labels for deterministic output is also a good touch.
95-107: Deduplicate lease info formatting via a small helperYou reconstruct (client, status, start_time) here; consider extracting a helper to avoid duplication and simplify tests. This aligns with prior feedback.
Apply this diff within this method:
def rich_add_rows(self, table, options: WithOptions = None): - lease_info = None - if options and options.show_leases and self.lease: - lease_client = self.lease.client - lease_status = self.lease.get_status() - start_time = "" - if self.lease.effective_begin_time: - start_time = self.lease.effective_begin_time.strftime("%Y-%m-%d %H:%M:%S") - lease_info = (lease_client, lease_status, start_time) - elif options and options.show_leases: - lease_info = ("", "Available", "") + lease_info = extract_lease_info(self.lease) if (options and options.show_leases) else None add_exporter_row(table, self, options, lease_info)And add this helper near the other display helpers:
def extract_lease_info(lease: "Lease | None") -> tuple[str, str, str]: if not lease: return ("", "Available", "") status = lease.get_status() start = lease.effective_begin_time.strftime("%Y-%m-%d %H:%M:%S") if lease.effective_begin_time else "" return (lease.client, status, start)
221-236: Resolved: no more leak of lease/online in JSON when excludedUsing a computed exclude set to omit lease/online is correct and addresses the earlier data exposure concern.
🧹 Nitpick comments (5)
packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py (3)
144-154: Improve error message clarity for invalid tokensPresenting Python list reprs in CLI errors is noisy. Quote and join invalid tokens for readability.
Apply this diff:
- allowed_list = ", ".join(sorted(allowed_values)) - raise click.BadParameter( - f"Invalid value(s) {invalid}. Allowed values are: {allowed_list}", - ctx=ctx, - param=param - ) + allowed_list = ", ".join(sorted(allowed_values)) + invalid_list = ", ".join(f"'{t}'" for t in invalid) + raise click.BadParameter( + f"Invalid value(s): {invalid_list}. Allowed values are: {allowed_list}", + ctx=ctx, + param=param + )
199-235: Polish the Click option UX (metavar and explicit type)Minor CLI ergonomics: add a metavar hint and explicitly set type=str for clarity. No functional change.
Apply this diff:
return click.option( f"--{name}", f"{name}_options", + type=str, callback=callback, multiple=True, + metavar="TOKEN[,TOKEN...]", help=help_text )
156-197: Normalize allowed_values when normalize_case=True in parse_comma_separatedTo avoid rejecting valid values when callers pass an uppercase/mixed‐case allowed_values set while normalize_case is enabled, update the validation step in packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py around the call to _validate_tokens:
@@ packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py - # Validate if allowed values are specified - if allowed_values is not None: - _validate_tokens(unique_tokens, allowed_values, ctx, param) + # Validate if allowed values are specified + if allowed_values is not None: + # Normalize allowed_values in sync with token normalization + allowed_norm = ( + {v.strip().lower() for v in allowed_values} + if normalize_case + else set(allowed_values) + ) + _validate_tokens(unique_tokens, allowed_norm, ctx, param)packages/jumpstarter/jumpstarter/client/grpc.py (2)
198-200: Minor naming nit: include_ vs show_**ExporterList uses include_online/include_leases while WithOptions uses show_online/show_leases. It’s harmless, but aligning the terminology (either “include_” or “show_” across both) would reduce cognitive overhead.
221-236: Document limited kwargs and consider a tiny DRY helper for exclude fieldsThese overrides intentionally accept only a subset of model_dump_json kwargs and silently ignore others. A short docstring will prevent confusion for callers. Also, the exclude_fields logic is duplicated in both methods.
Apply docstrings in both methods:
def model_dump_json(self, **kwargs): + """Dump ExporterList to a JSON string. + + Note: intentionally supports only a subset of kwargs: indent, separators, + sort_keys, ensure_ascii. Other kwargs are ignored. Fields 'lease' and + 'online' are conditionally excluded based on include_* flags. + """def model_dump(self, **kwargs): + """Dump ExporterList to a Python dict (JSON-compatible). + + Note: diverges from Pydantic's full model_dump signature; typical kwargs + like include/exclude are not supported. Fields 'lease' and 'online' are + conditionally excluded based on include_* flags. + """Optionally, factor out the exclude computation:
def _exclude_fields(self) -> set[str]: exclude = set() if not self.include_leases: exclude.add("lease") if not self.include_online: exclude.add("online") return excludeThen use in both methods:
exclude_fields = self._exclude_fields()Also applies to: 238-250
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py(1 hunks)packages/jumpstarter-cli/jumpstarter_cli/get.py(2 hunks)packages/jumpstarter-cli/jumpstarter_cli/get_test.py(1 hunks)packages/jumpstarter/jumpstarter/client/grpc.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/jumpstarter-cli/jumpstarter_cli/get_test.py
- packages/jumpstarter-cli/jumpstarter_cli/get.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py (2)
packages/jumpstarter-cli/jumpstarter_cli/get_test.py (1)
allowed_values(15-17)packages/jumpstarter-cli-common/jumpstarter_cli_common/config.py (1)
callback(22-24)
packages/jumpstarter/jumpstarter/client/grpc.py (3)
packages/jumpstarter/jumpstarter/common/metadata.py (1)
name(13-14)packages/jumpstarter/jumpstarter/client/lease.py (1)
Lease(31-211)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clients.py (6)
rich_add_columns(57-60)rich_add_columns(80-81)rich_add_rows(62-66)rich_add_rows(83-85)rich_add_names(68-69)rich_add_names(87-89)
⏰ 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). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: e2e
🔇 Additional comments (8)
packages/jumpstarter-cli-common/jumpstarter_cli_common/opt.py (2)
129-137: Tokenization and normalization look solidGood handling of whitespace, lowercasing, and ignoring empty tokens. The generator pipeline keeps this concise and efficient.
139-142: Deterministic de-duplication with order preservationUsing dict.fromkeys is a clean, Pythonic way to preserve order while removing duplicates.
packages/jumpstarter/jumpstarter/client/grpc.py (6)
19-23: Good introduction of display options containerThe WithOptions dataclass cleanly encapsulates the two toggles and keeps UI concerns decoupled from models.
25-36: Column gating looks correct and keeps output stableColumns are added deterministically and only when requested, which helps keep table layouts predictable across commands.
83-85: Exporter model: fields look appropriate and forward-ref safeAdding online and lease here makes sense; forward refs are covered by from future import annotations.
92-94: Nice delegation via add_display_columnsKeeps column definition centralized and consistent with the row builder.
208-211: Consistent propagation of options to columns and rowsThe pattern of constructing WithOptions once and delegating to Exporter methods is clean and keeps rendering logic centralized.
Also applies to: 213-216
86-90: Ensure protocol compatibility foronline
The newfrom_protobufreadsdata.online, which only exists if your gRPC protocol definitions (jumpstarter-protocol PR #24) have been applied. If the updated proto isn’t in use at runtime, you’ll get anAttributeError.Please verify:
- The
Exportermessage in your.protofiles declares anonlinefield (bool).- The generated
client_pb2.pyclassExporterdefinesonline.- Your jumpstarter-protocol dependency has been bumped to include PR #24 before deploying.
4 participants
requires jumpstarter-dev/jumpstarter-protocol#24 and jumpstarter-dev/jumpstarter-controller#154
Summary by CodeRabbit
New Features
Refactor
Tests