Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdded a new spotlight resource with sync/async wrappers exposing an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e/spotlight/test_async_spotlight.py`:
- Around line 28-33: The test test_refresh_spotlight_objects_specific assumes
spotlight_objects_data has elements and directly indexes
spotlight_objects_data[0]; add a guard to handle empty fixture data by checking
spotlight_objects_data before indexing (e.g., if not spotlight_objects_data:
pytest.skip("no spotlight objects seeded") or assert spotlight_objects_data,
"expected seeded spotlight objects"), then proceed to call
spotlight_objects_service.refresh(object_id=result.id) using the safely obtained
result; update references to spotlight_objects_data and
spotlight_objects_service.refresh in the test accordingly.
In `@tests/e2e/spotlight/test_sync_spotlight.py`:
- Around line 28-31: The test test_refresh_spotlight_objects_specific can raise
IndexError when spotlight_objects_data is empty; add a guard at the start to
check if spotlight_objects_data is empty and either call pytest.skip("no
spotlight seed data") or assert spotlight_objects_data, then proceed to fetch
result = spotlight_objects_data[0] and call
spotlight_objects_service.refresh(object_id=result.id). This uses the existing
symbols spotlight_objects_data and spotlight_objects_service.refresh to locate
where to insert the check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c92904c6-fa37-44d6-9be6-909961ce6607
📒 Files selected for processing (10)
mpt_api_client/mpt_client.pympt_api_client/resources/__init__.pympt_api_client/resources/spotlight/__init__.pympt_api_client/resources/spotlight/objects.pympt_api_client/resources/spotlight/spotlight.pytests/e2e/spotlight/test_async_spotlight.pytests/e2e/spotlight/test_sync_spotlight.pytests/unit/resources/spotlight/test_objects.pytests/unit/resources/spotlight/test_spotlight.pytests/unit/test_mpt_client.py
robcsegal
force-pushed
the
MPT-20429-add-endpoints-and-e-2-e-tests-for-spotlight-objects
branch
from
f438386 to
8e3055b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mpt_api_client/resources/spotlight/spotlight.py (1)
14-17: Consider caching service instances in properties.The
objectsproperty creates a new service instance on every access, which may be inefficient if accessed multiple times. However, this pattern is consistent with other resource properties in the codebase (e.g.,program,spotlightin MPTClient).If service instances are lightweight and stateless, the current approach is fine. Otherwise, consider caching:
`@property` def objects(self) -> SpotlightObjectsService: """Spotlight Objects service.""" if not hasattr(self, '_objects'): self._objects = SpotlightObjectsService(http_client=self.http_client) return self._objectsAlso applies to: 26-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mpt_api_client/resources/spotlight/spotlight.py` around lines 14 - 17, The objects property currently instantiates a new SpotlightObjectsService on each access (method objects) — change it to cache the instance on the Spotlight class (e.g., store in self._objects) so subsequent accesses return the same SpotlightObjectsService(http_client=self.http_client) instance; apply the same caching pattern to the other similar properties referenced (the ones creating SpotlightProgramsService/SpotlightService or similar) to avoid repeated construction while preserving the existing type hints and docstrings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mpt_api_client/resources/spotlight/spotlight.py`:
- Around line 14-17: The objects property currently instantiates a new
SpotlightObjectsService on each access (method objects) — change it to cache the
instance on the Spotlight class (e.g., store in self._objects) so subsequent
accesses return the same SpotlightObjectsService(http_client=self.http_client)
instance; apply the same caching pattern to the other similar properties
referenced (the ones creating SpotlightProgramsService/SpotlightService or
similar) to avoid repeated construction while preserving the existing type hints
and docstrings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: fbd0d1fb-d6d4-41a4-92b7-d654322e7be9
📒 Files selected for processing (10)
mpt_api_client/mpt_client.pympt_api_client/resources/__init__.pympt_api_client/resources/spotlight/__init__.pympt_api_client/resources/spotlight/objects.pympt_api_client/resources/spotlight/spotlight.pytests/e2e/spotlight/test_async_spotlight.pytests/e2e/spotlight/test_sync_spotlight.pytests/unit/resources/spotlight/test_objects.pytests/unit/resources/spotlight/test_spotlight.pytests/unit/test_mpt_client.py
✅ Files skipped from review due to trivial changes (4)
- tests/unit/test_mpt_client.py
- mpt_api_client/mpt_client.py
- tests/unit/resources/spotlight/test_spotlight.py
- mpt_api_client/resources/init.py
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/unit/resources/spotlight/test_objects.py
- tests/e2e/spotlight/test_sync_spotlight.py
- mpt_api_client/resources/spotlight/objects.py
- tests/e2e/spotlight/test_async_spotlight.py
robcsegal
changed the title
robcsegal
force-pushed
the
MPT-20429-add-endpoints-and-e-2-e-tests-for-spotlight-objects
branch
from
8e3055b to
11e2b75
Compare
|
d3rky
approved these changes
Apr 21, 2026
d3rky
deleted the
MPT-20429-add-endpoints-and-e-2-e-tests-for-spotlight-objects
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces the new
SpotlightAPI module to the client, providing both synchronous and asynchronous support for spotlight object resources. It includes full implementation of the resource, integration into the main API client, and comprehensive unit and end-to-end tests.Spotlight API Module Implementation:
SpotlightandAsyncSpotlightclasses inmpt_api_client/resources/spotlight/spotlight.py, exposing theobjectsproperty for managing spotlight objects.SpotlightObjectmodel,SpotlightObjectsService, andAsyncSpotlightObjectsServiceinmpt_api_client/resources/spotlight/objects.py, includingrefreshmethods for both sync and async usage.Integration with Main Client and Exports:
SpotlightandAsyncSpotlightin the mainMPTClientandAsyncMPTClientclasses inmpt_api_client/mpt_client.py, and updated exports inmpt_api_client/resources/__init__.pyandmpt_api_client/resources/spotlight/__init__.py. [1] [2] [3] [4] [5] [6] [7] [8]Testing:
SpotlightandAsyncSpotlightmodules and their service properties intests/unit/resources/spotlight/test_spotlight.py, and for theSpotlightObjectsServiceandAsyncSpotlightObjectsService(including therefreshmethod and mixin presence) intests/unit/resources/spotlight/test_objects.py. [1] [2]tests/e2e/spotlight/test_sync_spotlight.pyandtests/e2e/spotlight/test_async_spotlight.py. [1] [2]tests/unit/test_mpt_client.pyto includeSpotlightandAsyncSpotlightin client resource tests. [1] [2] [3] [4]Closes MPT-20429
Release Notes