Repository navigation
fix: flatten the pages in DeploymentResourcePool.query_deployed_models - #7231
Open
abhishekacharya200 wants to merge 1 commit into
Open
abhishekacharya200 wants to merge 1 commit into
abhishekacharya200 wants to merge 1 commit into
Conversation
itertools.chain was called with a single generator, so it did not flatten anything: the method returned one RepeatedComposite per page instead of the documented list of DeployedModelRef objects. Use itertools.chain.from_iterable in both the GA and preview classes.
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.
Fixes #7230 🦕
DeploymentResourcePool.query_deployed_models()is documented, and annotated, to return a list ofDeployedModelRefobjects, but it built that list withitertools.chain(page.deployed_model_refs for page in response.pages). Passing a single generator tochaindoes not flatten it, so the result held oneRepeatedCompositeper page instead of the refs themselves. With one page holding two refs the method returned a list of length 1, andrefs[0].deployed_model_idraisedAttributeError.This switches to
itertools.chain.from_iterable(...)in both the GA class (google/cloud/aiplatform/models.py) and the preview class (google/cloud/aiplatform/preview/models.py), so the method returns what its docstring and return type promise.Tests in
tests/unit/aiplatform/test_deployment_resource_pools.py:test_query_deployed_modelsnow checks the returneddeployed_model_idvalues, not only the length. The length check passed with the bug because the fixture has two pages with one ref each.test_query_deployed_models_with_several_refs_in_one_pagecovers one page holding two refs.test_preview_query_deployed_modelscovers the preview class.All three fail on
mainand pass with this change, and the whole file passes (12 passed).black==24.8.0andflake8==6.1.0from the noxfile report nothing on the changed files.With the script from #7230: