feat: add search by attached data asset for capsules and pipelines - #78
Open
arielleleon wants to merge 1 commit into
Open
feat: add search by attached data asset for capsules and pipelines#78arielleleon wants to merge 1 commit into
arielleleon wants to merge 1 commit into
Conversation
Add Capsules.search_capsules_by_data_asset and Pipelines.search_pipelines_by_data_asset, which answer "is this data asset attached to anything, and to what" by searching the capsules and pipelines routes for a data asset ID and collecting every page of matches. Uses the existing POST capsules/search and pipelines/search endpoints, so no Min-Server-Version change is required. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new public docstrings and one test fixture are slightly misleading/unclear and should be tightened for accuracy/readability before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a convenient, paginated “search by attached data asset” helper on the Capsules and Pipelines resource clients (closing #77), plus unit tests covering request routing and pagination behavior.
Changes:
- Add
Capsules.search_capsules_by_data_asset(data_asset_id) -> list[Capsule]implemented via the existing search iterator. - Add
Pipelines.search_pipelines_by_data_asset(data_asset_id) -> list[Capsule]as a one-line forwarder following the existing route-delegation pattern. - Add mock-session tests validating query placement, empty results, pagination, and the pipelines route.
File summaries
| File | Description |
|---|---|
src/codeocean/capsule.py |
Adds search_capsules_by_data_asset convenience wrapper around the existing search iterator. |
src/codeocean/pipeline.py |
Adds search_pipelines_by_data_asset forwarding helper to keep pipelines behavior in sync with capsules. |
tests/test_search_by_data_asset.py |
Adds mock-based tests for both capsule and pipeline variants, including pagination. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+130
to
+142
| """Find the capsules that currently have a given data asset attached. | ||
|
|
||
| Searches for the data asset ID across all capsules accessible to the caller, | ||
| following pagination until every match is collected. Results are whatever capsule | ||
| search reports for the ID, so an empty list means no capsule accessible to the | ||
| caller is reported as having the data asset attached. | ||
|
|
||
| Args: | ||
| data_asset_id: ID of the data asset to look for | ||
|
|
||
| Returns: | ||
| Capsules reported as having the data asset attached | ||
| """ |
Comment on lines
+83
to
+95
| """Find the pipelines that currently have a given data asset attached. | ||
|
|
||
| Searches for the data asset ID across all pipelines accessible to the caller, | ||
| following pagination until every match is collected. Results are whatever pipeline | ||
| search reports for the ID, so an empty list means no pipeline accessible to the | ||
| caller is reported as having the data asset attached. | ||
|
|
||
| Args: | ||
| data_asset_id: ID of the data asset to look for | ||
|
|
||
| Returns: | ||
| Pipelines reported as having the data asset attached | ||
| """ |
Comment on lines
+87
to
+96
| """The pipeline variant searches the pipelines route.""" | ||
| session = self._mock_session({"has_more": False, "results": [CAPSULE]}) | ||
| pipelines = Pipelines(client=session) | ||
|
|
||
| results = pipelines.search_pipelines_by_data_asset("asset-789") | ||
|
|
||
| [(route, body)] = self._posted(session) | ||
| self.assertEqual(route, "pipelines/search") | ||
| self.assertEqual(body["query"], "asset-789") | ||
| self.assertEqual([p.id for p in results], ["cap-123"]) |
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
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.
Closes #77.
What
Adds a convenience method to each resource client for finding the capsules or pipelines that
have a given data asset attached:
An empty list means the data asset is not attached to anything the caller can access, so one
call answers both "is it attached" and "what is it attached to". The main use case is checking
whether anything still depends on a data asset before archiving or deleting it.
Why
The pieces already exist, but the caller has to know that a bare data asset ID works as a
CapsuleSearchParams.querywhich isn't discoverable from that field's documentation andhas to drive pagination themselves, separately for capsules and pipelines.
Implementation notes
POST capsules/searchandPOST pipelines/searchendpoints, soMIN_SERVER_VERSIONis unchanged.search_capsules_iterator, so all pages are collected rather than just thefirst.
Capsules._routepattern, soPipelines.search_pipelines_by_data_assetis aone-line forwarder and both resources stay in sync.
CHANGELOG.mdor version changes, matching how other feature PRs here leave those to therelease PR. Happy to add a changelog entry if you'd rather have it in this PR.
Testing
tests/test_search_by_data_asset.pyfollows the mock-session style oftest_git_sync.pyandcovers: the data asset ID being sent as the search query on the correct route, the
not-attached case returning an empty list, pagination across multiple pages, and the pipeline
variant hitting
pipelines/search.The assertions check the route and the
query/next_tokenkeys rather than the whole requestbody, so adding fields to
CapsuleSearchParamslater won't break them.Tested locally on Python 3.11; the 3.9, 3.13 matrix is left to CI.
Verification against a live deployment
The unit tests are mock-based, so I also exercised the methods against a real 4.x deployment
(read-only calls) to check that a search for a data asset ID behaves like an attachment lookup:
5 capsules, and every one of them had a computation that had mounted that asset.
capsule, so the same
queryfield matches names/descriptions/tags as well as attachments.mounted the asset, 5 capsules came back from a search for that asset and 2 did not. That is
consistent with the asset having been detached from those capsules after those runs, which
would be the correct result for a "currently attached" lookup.
Open questions
Capsules/Pipelinesto keep each module alignedwith its route, but
client.data_assets.list_attached_capsules(...)may read better giventhe question starts from a data asset. Easy to move.
list[Capsule]for a straightforward truthiness check. I can addan
..._iteratorvariant instead, or as well, to match the existing search method pairs.capsule that currently has it attached, or can the index lag or miss cases? This decides how
strongly the method can be documented: the intended use is checking whether anything still
depends on a data asset before archiving it, and there an empty result that should not have
been empty is the costly direction. If the index isn't authoritative for this, I'm happy to
document the caveat, or to build the check on a different endpoint if there's a better one.