[GitHub] [airflow] eladkal commented on a change in pull request #5419: [AIRFLOW-XXXX] Update pydoc of mlengine_operator

[GitBox <gi...@apache.org>]

eladkal commented on a change in pull request #5419: [AIRFLOW-XXXX] Update pydoc of mlengine_operator
URL: https://github.com/apache/airflow/pull/5419#discussion_r294059123
 
 

 ##########
 File path: airflow/contrib/operators/mlengine_operator.py
 ##########
 @@ -135,7 +135,7 @@ class MLEngineBatchPredictionOperator(BaseOperator):
 
     :param max_worker_count: The maximum number of workers to be used
         for parallel processing. Defaults to 10 if not specified.
-    :type max_worker_count: int
+    :type max_worker_count: string
 
 Review comment:
   > The maxWorkerCount parameter in mlengine is, somewhat surprisingly, a string:
   
   Actually it make sense. There are two properties `Type `and `Format`.
   Some parameters like the maxWorkerCount require both  to determine the data type of the property.  Type is `String `and Format is `INT64` 
   
   The reason for this is explained in the docs:
   
   > The type property indicates the type of the property when its sent in JSON requests and responses (JSON supports a small set of data types, see json.org for details). The format property provides additional information about the underlying type. Properties will always have a type property, but some may also have a format property.
   > 
   > For example, a 64-bit integer cannot be represented in JSON (since JavaScript and JSON support integers up to 2^53). Therefore, a 64-bit integer must be represented as a string in JSON requests/responses. So the type property will be set to "string", but the format property will be set to "int64" to indicate that it is a 64-bit integer.
   > 
   
   https://developers.google.com/discovery/v1/type-format
   
   The pydoc should mention both `Type `and `Format `otherwise users will be confused.
   
   idealy it would be nice to have:
   ```
   :param max_worker_count: ....
   :type max_worker_count: string
   :format max_worker_count: int
   ```
   But If that is unacceptable how about:
   `:type max_worker_count: string  (Format int)
   `

----------------------------------------------------------------
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


With regards,
Apache Git Services