feat: support raw_predict for Endpoint #1620

rosiezou · 2022-08-30T01:42:33Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…platform into raw-predict

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…platform into raw-predict

nayaknishant

Is test_model_interactions.py testing permanent models? If it is I think we should keep it as two separate files (as you have it), but if it's using temp models created in test_model_upload.py wouldn't it make more sense to have a predict() call and then a raw_predict() call after in the same file (maybe change the name to a more broad test_model.py ?

nayaknishant · 2022-08-31T23:23:55Z

google/cloud/aiplatform/models.py

+            return Prediction(
+                predictions=response_text["predictions"],
+                deployed_model_id=raw_predict_response.headers[
+                    "X-Vertex-AI-Deployed-Model-Id"


Interesting, if you can clarify @rosiezou, I'm guessing X-Vertex-AI-Deployed-Model-Id is populated when the response is returned?

@nayaknishant Indeed, this is documented here: https://github.com/googleapis/googleapis/blob/19bd392eedec5c877f1241ee00d6b2cb34cfadc2/google/cloud/aiplatform/v1/prediction_service.proto#L56

I also added constants (_RAW_PREDICT_MODEL_ID_KEY = "X-Vertex-AI-Deployed-Model-Id" and _RAW_PREDICT_MODEL_RESOURCE_KEY = "X-Vertex-AI-Model") to replace the hardcoded strings

nayaknishant

I see that system tests have been added, but can we add also unit tests to https://github.com/googleapis/python-aiplatform/blob/main/tests/unit/aiplatform/test_models.py for raw_predict() to ensure the function works as intended during future development.

google/cloud/aiplatform/constants/base.py

dizcology · 2022-09-01T17:52:20Z

google/cloud/aiplatform/models.py

@@ -35,9 +36,11 @@
 from google.api_core import operation
 from google.api_core import exceptions as api_exceptions
 from google.auth import credentials as auth_credentials
+from google.auth.transport.requests import AuthorizedSession


Import module instead of class. Because of the import name conflict maybe something like from google.auth.transport import requests as google_auth_requests.

updated import statement

dizcology · 2022-09-01T17:54:03Z

google/cloud/aiplatform/models.py

@@ -1505,29 +1511,67 @@ def predict(
                [PredictSchemata's][google.cloud.aiplatform.v1beta1.Model.predict_schemata]
                ``parameters_schema_uri``.
            timeout (float): Optional. The timeout for this request in seconds.
+            use_raw_predict (bool):
+                Optional. If set to True, the underlying prediction call will be made
+                against Endpoint.raw_predict(). Currently, model version information will


Maybe mention that the default is False in the docstring.

"Currently..." <- does this mean that later it will become available?

I will remove "currently". I have checked with the service team and they mentioned that it's not on the roadmap to make model version info available for raw_predict due to performance reasons.

updated doc strings

dizcology · 2022-09-01T17:55:43Z

google/cloud/aiplatform/models.py

+            return Prediction(
+                predictions=response_text["predictions"],
+                deployed_model_id=raw_predict_response.headers[
+                    "X-Vertex-AI-Deployed-Model-Id"


@nayaknishant Indeed, this is documented here: https://github.com/googleapis/googleapis/blob/19bd392eedec5c877f1241ee00d6b2cb34cfadc2/google/cloud/aiplatform/v1/prediction_service.proto#L56

dizcology · 2022-09-01T17:57:03Z

google/cloud/aiplatform/models.py

+                body=json.dumps({"instances": instances, "parameters": parameters}),
+                headers={"Content-Type": "application/json"},
+            )
+            response_text = json.loads(raw_predict_response.text)


After calling json.loads this is no longer a text. Please rename the variable response_text to something else.

updated variable name

dizcology · 2022-09-01T17:58:20Z

google/cloud/aiplatform/models.py

+            body (bytes):
+                Required. The body of the prediction request in bytes. This must not exceed 1.5 mb per request.
+            headers (Dict[str, str]):
+                Required. The header of the request as a dictionary. There are no restrictions on the header.


There is a default value so this is not actually required, contrary to what the docstring says.

On the other hand the body argument probably should not have a default?

You're right, neither of these args should have any defaults. I've updated the function signatures and doc strings.

dizcology · 2022-09-01T17:58:46Z

google/cloud/aiplatform/models.py

-            model_resource_name=prediction_response.model,
-        )
+    def raw_predict(
+        self, body: bytes = None, headers: Dict[str, str] = None


If we are going to allow None, the typing annotation should be Optional[Dict[str, str]].

dizcology · 2022-09-01T18:00:30Z

google/cloud/aiplatform/models.py

@@ -2062,6 +2106,30 @@ def predict(self, instances: List, parameters: Optional[Dict] = None) -> Predict
            deployed_model_id=self._gca_resource.deployed_models[0].id,
        )

+    def raw_predict(
+        self, body: bytes = None, headers: Dict[str, str] = None


Same comments about the arguments' default values here.

dizcology · 2022-09-01T18:02:50Z

google/cloud/aiplatform/models.py

+    ) -> requests.models.Response:
+        """Make a prediction request using arbitrary headers.
+        This method must be called within the network the PrivateEndpoint is peered to.
+        The function call will fail otherwise. To check, use `PrivateEndpoint.network`.


Do we know the specific error that would be raised? If so perhaps add a Raises: section to the docstring: https://google.github.io/styleguide/pyguide.html#doc-function-raises

I'll test out a few scenarios, but most common one will be auth errors raised from google.api_core.exceptions with error code 401

I meant specifically the part “the function call will fail” in the docstring.

Got it. I updated the doc strings in PrivateEndpoint.predict and PrivateEndpoint.raw_predict to contain information about the error code. Most common error code will be 404 with a message saying "request not found".

dizcology · 2022-09-01T18:05:29Z

tests/system/aiplatform/test_model_interactions.py

+            {"Content-Type": "application/json"},
+        )
+        assert raw_prediction_response.status_code == 200
+        assert len(json.loads(raw_prediction_response.text).items()) == 1


.items() is not needed if we are only checking the dictionary's length.

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

dizcology · 2022-09-02T03:10:46Z

google/cloud/aiplatform/models.py

@@ -69,6 +72,8 @@
 _DEFAULT_MACHINE_TYPE = "n1-standard-2"
 _DEPLOYING_MODEL_TRAFFIC_SPLIT_KEY = "0"
 _SUCCESSFUL_HTTP_RESPONSE = 300
+_RAW_PREDICT_MODEL_ID_KEY = "X-Vertex-AI-Deployed-Model-Id"


Probably call this ”deployed model id” instead of “model id” (which sometimes refers to the last part of the resource full name).

updated variable name

Ark-kun · 2023-10-18T22:38:59Z

google/cloud/aiplatform/models.py

+        self.authorized_session = None
+        self.raw_predict_request_url = None


Should these attributes be public?

Ark-kun · 2023-10-18T22:39:58Z

google/cloud/aiplatform/base.py

+            "_authorized_session",
+            "_raw_predict_request_url",


These attribute names start with underscores, but the attributes in models.py do not have leading underscores. Is this correct?

rosiezou added 2 commits August 29, 2022 18:32

feat: support raw_predict for Endpoints

b42a47f

formatting

9f850b2

rosiezou requested review from a team as code owners August 30, 2022 01:42

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 30, 2022

Merge branch 'main' into raw-predict

5e32858

rosiezou requested review from nayaknishant and dizcology August 30, 2022 01:43

rosiezou self-assigned this Aug 30, 2022

rosiezou and others added 3 commits August 30, 2022 22:00

fixed broken unit test

f6e7241

Merge branch 'main' into raw-predict

07796ae

🦉 Updates from OwlBot post-processor

5df7af8

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

gcf-owl-bot bot requested review from a team as code owners August 31, 2022 05:05

gcf-owl-bot bot requested a review from dandhlee August 31, 2022 05:05

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Aug 31, 2022

gcf-owl-bot bot added 3 commits August 31, 2022 05:05

🦉 Updates from OwlBot post-processor

3ed3e06

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'raw-predict' of https://github.com/googleapis/python-ai…

516771e

…platform into raw-predict

🦉 Updates from OwlBot post-processor

4586da2

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

product-auto-label bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels Aug 31, 2022

gcf-owl-bot bot and others added 4 commits August 31, 2022 05:08

🦉 Updates from OwlBot post-processor

f5c7dea

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'raw-predict' of https://github.com/googleapis/python-ai…

007595f

…platform into raw-predict

Merge branch 'main' into raw-predict

ffd5a75

remove commented out code blocks

4dd2472

nayaknishant reviewed Aug 31, 2022

View reviewed changes

rosiezou added 2 commits August 31, 2022 14:20

removing debug print statements

3818bf6

removing extra prints

ca30449

nayaknishant reviewed Aug 31, 2022

View reviewed changes

Merge branch 'main' into raw-predict

267e803

dizcology requested changes Sep 1, 2022

View reviewed changes

rosiezou and others added 7 commits September 1, 2022 12:32

Merge branch 'main' into raw-predict

474654c

Merge branch 'main' into raw-predict

f2386c3

addressed PR comments

60ea5f9

🦉 Updates from OwlBot post-processor

ee319fd

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

adding unit test

eef9e6f

🦉 Updates from OwlBot post-processor

19c853e

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

removed unused import

bfac52c

dizcology reviewed Sep 2, 2022

View reviewed changes

rosiezou and others added 3 commits September 2, 2022 00:30

Merge branch 'main' into raw-predict

d6e8ef7

renamed raw predict constants

2175a2c

modified error messages

748b0ed

dizcology approved these changes Sep 2, 2022

View reviewed changes

added doc strings

bc80e81

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Sep 2, 2022

rosiezou added 2 commits September 2, 2022 13:00

fixed typo in doc strings

f0f1e7a

removed extra space

be8d4de

nayaknishant approved these changes Sep 2, 2022

View reviewed changes

Merge branch 'main' into raw-predict

1264f33

rosiezou merged commit cc7c968 into main Sep 2, 2022

rosiezou deleted the raw-predict branch September 2, 2022 22:34

release-please bot mentioned this pull request Sep 2, 2022

chore(main): release 1.17.0 #1573

Merged

release-please bot mentioned this pull request Jun 8, 2023

chore(main): release 1.24.1 #2196

Closed

release-please bot mentioned this pull request Aug 25, 2023

chore(main): release 1.30.1 #2490

Closed

release-please bot mentioned this pull request Sep 5, 2023

chore(main): release 1.32.0 #2548

Merged

Ark-kun reviewed Oct 18, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support raw_predict for Endpoint #1620

feat: support raw_predict for Endpoint #1620

rosiezou commented Aug 30, 2022

nayaknishant left a comment

nayaknishant Aug 31, 2022

dizcology Sep 1, 2022

rosiezou Sep 1, 2022

nayaknishant left a comment

dizcology Sep 1, 2022

rosiezou Sep 1, 2022

dizcology Sep 1, 2022

rosiezou Sep 1, 2022

rosiezou Sep 1, 2022

dizcology Sep 1, 2022

dizcology Sep 1, 2022

rosiezou Sep 1, 2022

dizcology Sep 1, 2022

rosiezou Sep 1, 2022

dizcology Sep 1, 2022

dizcology Sep 1, 2022

dizcology Sep 1, 2022

rosiezou Sep 1, 2022

dizcology Sep 2, 2022

rosiezou Sep 2, 2022

dizcology Sep 1, 2022

rosiezou Sep 1, 2022

dizcology Sep 2, 2022

rosiezou Sep 2, 2022

Ark-kun Oct 18, 2023

Ark-kun Oct 18, 2023

		self.authorized_session = None
		self.raw_predict_request_url = None

feat: support raw_predict for Endpoint #1620

feat: support raw_predict for Endpoint #1620

Conversation

rosiezou commented Aug 30, 2022

nayaknishant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nayaknishant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment