Conversation
There was a problem hiding this comment.
Pull Request Overview
Add a minimal fake lease client and wire it into FakeSpot to support lease-aware workflows in tests and simulations.
- Introduces LeaseOwner and Lease dataclasses
- Adds FakeLeaseClient with list_leases()
- Wires lease_client into FakeSpot initialization
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def list_leases(self): | ||
| lease_owner = LeaseOwner("fake_spot_lease_owner") | ||
| lease = [Lease(lease_owner)] | ||
| return [lease] |
There was a problem hiding this comment.
list_leases() currently returns a nested list (List[List[Lease]]) due to wrapping the already-list 'lease' in another list. Return a flat list instead, e.g., either 'return lease' or construct and return a single list in one step.
| return [lease] | |
| return lease |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| class FakeLeaseClient: | ||
| def __init__(self, fake_spot): | ||
| self.fake_spot = fake_spot | ||
|
|
||
| def list_leases(self): | ||
| lease_owner = LeaseOwner("fake_spot_lease_owner") | ||
| lease = [Lease(lease_owner)] | ||
| return lease |
There was a problem hiding this comment.
[nitpick] list_leases returns a bare list of Lease objects, which diverges from the typical Spot LeaseClient API that returns a response object with a resources collection. To make this fake a drop-in replacement for callers expecting the real client, return an object with a resources attribute (e.g., a simple dataclass wrapper like ListLeasesResponse with resources: list[LeaseResource]) or match whatever structure your production code consumes.
| @dataclass | ||
| class LeaseOwner: | ||
| client_name: str | ||
|
|
||
|
|
||
| @dataclass |
There was a problem hiding this comment.
[nitpick] Consider making these dataclasses immutable to prevent accidental mutation of lease metadata in tests: use @DataClass(frozen=True). Immutability better reflects how lease metadata is typically treated and avoids hard-to-track side effects.
| @dataclass | |
| class LeaseOwner: | |
| client_name: str | |
| @dataclass | |
| @dataclass(frozen=True) | |
| class LeaseOwner: | |
| client_name: str | |
| @dataclass(frozen=True) |
3 participants
No description provided.