Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add enable_simple_view to PipelineJob.list() #1614

Merged
merged 16 commits into from
Sep 14, 2022

Conversation

sararob
Copy link
Contributor

@sararob sararob commented Aug 25, 2022

Add support for read_mask option in PipelineJob.list() to improve performance. This will reduce the number of fields returned in the response for each PipelineJob.

@sararob sararob requested a review from a team as a code owner August 25, 2022 21:00
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: vertex-ai Issues related to the googleapis/python-aiplatform API. labels Aug 25, 2022
@sararob sararob marked this pull request as draft August 25, 2022 21:00
@sararob sararob marked this pull request as ready for review August 29, 2022 14:30
"job_detail.pipeline_run_context",
"job_detail.pipeline_context",
]
)
Copy link

Choose a reason for hiding this comment

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

nit - consider creating the path list as a constant somewhere that can be shared with the test file. Feel free to ignore if it needs too much refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@hilcj
Copy link

hilcj commented Aug 29, 2022

/lgtm

@sararob sararob requested a review from jaycee-li August 29, 2022 17:17
@@ -1026,6 +1027,7 @@ def _list(
cls_filter: Callable[[proto.Message], bool] = lambda _: True,
filter: Optional[str] = None,
order_by: Optional[str] = None,
read_mask: Optional[field_mask.FieldMask] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add doc string for this and the one in the next method?

enable_simple_view (bool):
Optional. Whether to pass the `read_mask` parameter to the list call.
This will improve the performance of calling list(). However, the
returned PipelineJob list will not include all fields for each PipelineJob.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think here we can specify what fields are missing.

@sararob sararob requested a review from jaycee-li August 29, 2022 20:25
@rosiezou rosiezou added do not merge Indicates a pull request not ready for merge, due to either quality or timing. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Sep 8, 2022
@sararob sararob merged commit 627fdf9 into googleapis:main Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: vertex-ai Issues related to the googleapis/python-aiplatform API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants