You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/08/07 15:02:10 UTC

[GitHub] [airflow] coopergillan opened a new pull request #10222: Add labels param to Google MLEngine Operators

coopergillan opened a new pull request #10222:
URL: https://github.com/apache/airflow/pull/10222


   Pass the `labels` parameter to the training jobs for two Google Machine Learning Operators. This will bring each of them up to snuff with `BigQueryExecuteQueryOperator`, which already accepts this parameter.
   
   closes: #10165


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] coopergillan commented on pull request #10222: Add labels param to Google MLEngine Operators

Posted by GitBox <gi...@apache.org>.
coopergillan commented on pull request #10222:
URL: https://github.com/apache/airflow/pull/10222#issuecomment-670565856


   @potiuk - is something like 57ed781b9b315e6e35a25f23f8a3f0a55ced304c sufficient?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] coopergillan commented on a change in pull request #10222: Add labels param to Google MLEngine Operators

Posted by GitBox <gi...@apache.org>.
coopergillan commented on a change in pull request #10222:
URL: https://github.com/apache/airflow/pull/10222#discussion_r467125875



##########
File path: airflow/providers/google/cloud/operators/mlengine.py
##########
@@ -1057,6 +1069,9 @@ def execute(self, context):
             self.log.info('MLEngine Training job request is: %s', training_request)
             return
 
+        if self._labels:
+            training_request['trainingInput']['labels'] = self._labels

Review comment:
       Thanks for the links. I will take a look and come back with some changes.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] coopergillan commented on pull request #10222: Add labels param to Google MLEngine Operators

Posted by GitBox <gi...@apache.org>.
coopergillan commented on pull request #10222:
URL: https://github.com/apache/airflow/pull/10222#issuecomment-670716820


   @mik-laj - thank you **very much** for those docs. I think I have this put together better in the last two commits here. Appreciate your help!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] coopergillan commented on a change in pull request #10222: Add labels param to Google MLEngine Operators

Posted by GitBox <gi...@apache.org>.
coopergillan commented on a change in pull request #10222:
URL: https://github.com/apache/airflow/pull/10222#discussion_r467336846



##########
File path: airflow/providers/google/cloud/operators/mlengine.py
##########
@@ -183,6 +185,7 @@ def __init__(self,  # pylint: disable=too-many-arguments
                  project_id: Optional[str] = None,
                  gcp_conn_id: str = 'google_cloud_default',
                  delegate_to: Optional[str] = None,
+                 labels: Optional[Dict[str, str]] = None,

Review comment:
       I'd like to come back through here and update the other `dict` types with more detail like this.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on a change in pull request #10222: Add labels param to Google MLEngine Operators

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10222:
URL: https://github.com/apache/airflow/pull/10222#discussion_r467109354



##########
File path: airflow/providers/google/cloud/operators/mlengine.py
##########
@@ -1057,6 +1069,9 @@ def execute(self, context):
             self.log.info('MLEngine Training job request is: %s', training_request)
             return
 
+        if self._labels:
+            training_request['trainingInput']['labels'] = self._labels

Review comment:
       I have the impression that the labels should be in ``training_request`, not ``trainingInput``. From what I can see, the API reference documentation for this product is incomplete and the description for the training_input field is missing, 
   https://cloud.google.com/ai-platform/prediction/docs/reference/rest/v1/projects.jobs#Job
   so you have to look at the Discovery documentation which gets updated more frequently so you have to look at the Discovery documentation, which gets more frequent updates, but also has bugs and missing parameter descriptions.
   https://ml.googleapis.com/$discovery/rest?version=v1
   More info about Discovery docs: https://developers.google.com/discovery
   
   <img width="773" alt="Screenshot 2020-08-07 at 17 24 00" src="https://user-images.githubusercontent.com/12058428/89661304-cd75c280-d8d2-11ea-940a-98c9c1bf04f3.png">
    




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj merged pull request #10222: Add labels param to Google MLEngine Operators

Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #10222:
URL: https://github.com/apache/airflow/pull/10222


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org