[GitHub] [airflow] mik-laj commented on pull request #8808: Support YAML input for CloudBuildCreateOperator

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

mik-laj commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-626451844


   I tried to implement this support based on template_ext. What do you think?
   ```diff
   05:31 $ git diff --cached
   diff --git a/airflow/providers/google/cloud/operators/cloud_build.py b/airflow/providers/google/cloud/operators/cloud_build.py
   index b86b45d12..739281c00 100644
   --- a/airflow/providers/google/cloud/operators/cloud_build.py
   +++ b/airflow/providers/google/cloud/operators/cloud_build.py
   @@ -16,6 +16,7 @@
    # specific language governing permissions and limitations
    # under the License.
    """Operators that integrat with Google Cloud Build service."""
   +import json
    import re
    from copy import deepcopy
    from typing import Any, Dict, Iterable, Optional, Union
   @@ -85,17 +86,6 @@ class BuildProcessor:
   
            self.body["source"]["storageSource"] = self._convert_storage_url_to_dict(source)
   
   -    def _load_body_to_dict(self):
   -        """
   -        :param file:
   -            file path to YAML build config
   -        :return: dict
   -        """
   -        try:
   -            with open(self.body, 'r') as f:
   -                self.body = yaml.safe_load(f)
   -        except yaml.YAMLError as e:
   -            raise AirflowException("Exception when loading resource definition: %s\n" % e)
   
        def process_body(self):
            """
   @@ -104,10 +94,6 @@ class BuildProcessor:
            :return: the body.
            :type: dict
            """
   -
   -        if isinstance(self.body, str):
   -            self._load_body_to_dict()
   -            return self.body
            self._verify_source()
            self._reformat_source()
            return self.body
   @@ -193,6 +179,7 @@ class CloudBuildCreateOperator(BaseOperator):
        """
   
        template_fields = ("body", "gcp_conn_id", "api_version")  # type: Iterable[str]
   +    template_ext = ['.yml', '.yaml', '.json']
   
        @apply_defaults
        def __init__(self,
   @@ -203,11 +190,22 @@ class CloudBuildCreateOperator(BaseOperator):
                     *args, **kwargs) -> None:
            super().__init__(*args, **kwargs)
            self.body = body
   +        # Not template fields to keep original value
   +        self.body_raw = body
            self.project_id = project_id
            self.gcp_conn_id = gcp_conn_id
            self.api_version = api_version
            self._validate_inputs()
   
   +    def prepare_template(self) -> None:
   +        # If no file is specified, skip
   +        if not isinstance(self.body_raw, str):
   +            return
   +        if any(self.body_raw.endswith(ext) for ext in ['.yml', '.yaml']):
   +            self.body = yaml.load(self.body)
   +        if self.body_raw.endswith('.json'):
   +            self.body = json.loads(self.body)
   +
        def _validate_inputs(self):
            if not self.body:
                raise AirflowException("The required parameter 'body' is missing")
   ```
   If you agree with this idea, could you add tests to this PR?
   


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