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/05/10 20:43:32 UTC
[GitHub] [airflow] joppevos opened a new pull request #8808: Support YAML input for CloudBuildCreateOperator
joppevos opened a new pull request #8808:
URL: https://github.com/apache/airflow/pull/8808
addresses the (following)[https://github.com/apache/airflow/issues/8567#issuecomment-620212297] issue.
---
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Target Github ISSUE in description if exists
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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] joppevos commented on a change in pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
joppevos commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r427489467
##########
File path: airflow/providers/google/cloud/operators/cloud_build.py
##########
@@ -175,21 +180,34 @@ class CloudBuildCreateOperator(BaseOperator):
"""
template_fields = ("body", "gcp_conn_id", "api_version") # type: Iterable[str]
+ template_ext = ['.yml', '.yaml', '.json']
@apply_defaults
def __init__(self,
- body: dict,
+ body: Union[dict, str],
Review comment:
Your right, I missed it. It is added now
----------------------------------------------------------------
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 pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-628493489
@joppevos Before calling the execute method, you must call the dag.resolve_template_files() method.
----------------------------------------------------------------
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 pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-629596799
@joppevos Github action is sad. Can you fix it?
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r423000840
##########
File path: airflow/providers/google/cloud/example_dags/example_cloud_build.yaml
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+steps:
Review comment:
We should allow the build object to be passed without sources.
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r422757560
##########
File path: airflow/providers/google/cloud/example_dags/example_cloud_build.py
##########
@@ -99,6 +99,12 @@
task_id="create_build_from_repo_result",
)
+ create_build_from_file = CloudBuildCreateOperator(
+ task_id="create_build_from_file", project_id=GCP_PROJECT_ID, body='example_cloud_build.yaml'
+ )
+
create_build_from_storage >> create_build_from_storage_result # pylint: disable=pointless-statement
create_build_from_repo >> create_build_from_repo_result # pylint: disable=pointless-statement
+
+ create_build_from_file # pylint: disable=pointless-statement
Review comment:
```suggestion
```
It is not necessary.
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r422757972
##########
File path: airflow/providers/google/cloud/example_dags/example_cloud_build.py
##########
@@ -99,6 +99,12 @@
task_id="create_build_from_repo_result",
)
+ create_build_from_file = CloudBuildCreateOperator(
Review comment:
Can you add this example to guide? https://airflow.readthedocs.io/en/latest/howto/operator/gcp/cloud_build.html
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r422760527
##########
File path: airflow/providers/google/cloud/example_dags/example_cloud_build.yaml
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+steps:
Review comment:
```
Traceback (most recent call last):
File "/usr/local/bin/airflow", line 11, in <module>
load_entry_point('apache-airflow', 'console_scripts', 'airflow')()
File "/opt/airflow/airflow/__main__.py", line 40, in main
args.func(args)
File "/opt/airflow/airflow/cli/cli_parser.py", line 52, in command
return func(*args, **kwargs)
File "/opt/airflow/airflow/utils/cli.py", line 84, in wrapper
return f(*args, **kwargs)
File "/opt/airflow/airflow/cli/commands/task_command.py", line 341, in task_test
ti.run(ignore_task_deps=True, ignore_ti_state=True, test_mode=True)
File "/opt/airflow/airflow/utils/session.py", line 61, in wrapper
return func(*args, **kwargs)
File "/opt/airflow/airflow/models/taskinstance.py", line 1143, in run
session=session)
File "/opt/airflow/airflow/utils/session.py", line 57, in wrapper
return func(*args, **kwargs)
File "/opt/airflow/airflow/sentry.py", line 140, in wrapper
return func(task_instance, *args, session=session, **kwargs)
File "/opt/airflow/airflow/models/taskinstance.py", line 1024, in _run_raw_task
result = task_copy.execute(context=context)
File "/opt/airflow/airflow/providers/google/cloud/operators/cloud_build.py", line 217, in execute
body = BuildProcessor(body=self.body).process_body()
File "/opt/airflow/airflow/providers/google/cloud/operators/cloud_build.py", line 99, in process_body
self._verify_source()
File "/opt/airflow/airflow/providers/google/cloud/operators/cloud_build.py", line 52, in _verify_source
is_storage = "storageSource" in self.body["source"]
KeyError: 'source'
```
I'm afraid this file won't work. Can you make this optional key?
Can you make 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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r422757102
##########
File path: airflow/providers/google/cloud/operators/cloud_build.py
##########
@@ -83,13 +85,29 @@ def _reformat_storage_source(self):
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:
Review comment:
I think, we should use [template_ext](https://airflow.readthedocs.io/en/latest/_api/airflow/models/baseoperator/index.html?highlight=BaseOperator#airflow.models.baseoperator.BaseOperator.template_ext) to load file. This will cause us to still have text instead of the dictionary, so we need to overwrite the [prepare_template method](https://airflow.readthedocs.io/en/latest/_api/airflow/models/baseoperator/index.html?highlight=BaseOperator#airflow.models.baseoperator.BaseOperator.prepare_template) to convert the string to a dictionary
----------------------------------------------------------------
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] joppevos commented on a change in pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
joppevos commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r422961945
##########
File path: airflow/providers/google/cloud/example_dags/example_cloud_build.yaml
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+steps:
Review comment:
providing a source is optional for a build config. Should we always check and fail if the body misses a source?
----------------------------------------------------------------
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] joppevos commented on pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
joppevos commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-630016214
@mik-laj You are helping me at every step ;) thanks. It seems that the `example_build.yaml` cannot be found. Currently, the file is in the same folder as the example. Should it get an absolute path to be found in the tests?
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r427014320
##########
File path: airflow/providers/google/cloud/example_dags/example_cloud_build.py
##########
@@ -99,6 +102,12 @@
task_id="create_build_from_repo_result",
)
+ # [START howto_operator_gcp_create_build_from_yaml_body]
+ create_build_from_file = CloudBuildCreateOperator(
+ task_id="create_build_from_file", project_id=GCP_PROJECT_ID,
+ body=str(CURRENT_FOLDER.joinpath('example_cloud_build.yaml'))
+ )
+ # [END howto_operator_gcp_create_build_from_yaml_body]
Review comment:
```suggestion
create_build_from_file = CloudBuildCreateOperator(
task_id="create_build_from_file",
project_id=GCP_PROJECT_ID,
body=str(CURRENT_FOLDER.joinpath('example_cloud_build.yaml')),
params={'name': 'Airflow'},
)
# [END howto_operator_gcp_create_build_from_yaml_body]
```
----------------------------------------------------------------
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 pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-629999249
Travis is sad. Do you need any 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] mik-laj edited a comment on pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-628493489
@joppevos It looks good for me, but before calling the execute method, you must call the dag.resolve_template_files() method.
----------------------------------------------------------------
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] joppevos commented on pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
joppevos commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-626640062
@mik-laj that's a great implementation. I was not aware of prepare_template but is it suited for it.
Should I take over your implementation in the PR? I will also add the rest of the feedback
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r427146223
##########
File path: airflow/providers/google/cloud/operators/cloud_build.py
##########
@@ -175,21 +180,34 @@ class CloudBuildCreateOperator(BaseOperator):
"""
template_fields = ("body", "gcp_conn_id", "api_version") # type: Iterable[str]
+ template_ext = ['.yml', '.yaml', '.json']
@apply_defaults
def __init__(self,
- body: dict,
+ body: Union[dict, str],
Review comment:
Can you also update the docstring? There should be information that the request body or file path can be passed as a parameter.
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r422757468
##########
File path: airflow/providers/google/cloud/operators/cloud_build.py
##########
@@ -83,13 +85,29 @@ def _reformat_storage_source(self):
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
Review comment:
Can you also add JSON file support? This is not common, but build can also be defined as JSON files.
> You can write the build config file using the YAML or the JSON syntax.
https://cloud.google.com/cloud-build/docs/build-config#structure_of_a_build_config_file
----------------------------------------------------------------
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 edited a comment on pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-626451844
I tried to implement this feature 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
[GitHub] [airflow] joppevos commented on pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
joppevos commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-628489425
Added the feedback. The only thing I have to make now is the test. I assume the test would check if it is able to work correctly given a YAML or JSON build config. How would I approach this? Something like the code below? New to testing to be honest.
I appreciate the given help @mik-laj.
```
@mock.patch("airflow.providers.google.cloud.operators.cloud_build.CloudBuildHook")
def test_load_yaml(self, hook_mock):
with tempfile.NamedTemporaryFile(suffix='.yaml', mode='w+t') as build:
build.writelines("""
steps:
- name: 'ubuntu'
args: ['echo', 'hello world, the day is {{ ds }} ']
""")
body_path = build.name
operator = CloudBuildCreateOperator(
body=body_path, project_id=TEST_PROJECT_ID, task_id="task-id"
)
return_value = operator.execute({})
# TODO: assert given yaml values are same as return
```
----------------------------------------------------------------
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] joppevos commented on pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
joppevos commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-630329569
@mik-laj The absolute path did indeed the trick. Build is passing now :thumbsup:
----------------------------------------------------------------
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 pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by 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
[GitHub] [airflow] mik-laj merged pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #8808:
URL: https://github.com/apache/airflow/pull/8808
----------------------------------------------------------------
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 pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-626666665
@joppevos Yes. Go ahead and use it.
----------------------------------------------------------------
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 pull request #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#issuecomment-630024504
@joppevos Yes. Here is example: https://github.com/apache/airflow/issues/8527
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r426420203
##########
File path: docs/howto/operator/gcp/cloud_build.rst
##########
@@ -80,6 +82,13 @@ It is also possible to specify it using the URL:
:start-after: [START howto_operator_gcp_cloud_build_source_repo_url]
:end-before: [END howto_operator_gcp_cloud_build_source_repo_url]
+It is also possible to specify it using a YAML or JSON format.
+.. exampleinclude:: ../../../../airflow/providers/google/cloud/example_dags/example_cloud_build.py
Review comment:
```suggestion
.. exampleinclude:: ../../../../airflow/providers/google/cloud/example_dags/example_cloud_build.py
```
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r422757760
##########
File path: airflow/providers/google/cloud/example_dags/example_cloud_build.yaml
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+steps:
+ - name: 'ubuntu'
+ args: ['echo', 'hello world']
Review comment:
Can you use jinja macro in this file? https://airflow.readthedocs.io/en/latest/macros-ref.html#default-variables
----------------------------------------------------------------
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 #8808: Support YAML input for CloudBuildCreateOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8808:
URL: https://github.com/apache/airflow/pull/8808#discussion_r427014320
##########
File path: airflow/providers/google/cloud/example_dags/example_cloud_build.py
##########
@@ -99,6 +102,12 @@
task_id="create_build_from_repo_result",
)
+ # [START howto_operator_gcp_create_build_from_yaml_body]
+ create_build_from_file = CloudBuildCreateOperator(
+ task_id="create_build_from_file", project_id=GCP_PROJECT_ID,
+ body=str(CURRENT_FOLDER.joinpath('example_cloud_build.yaml'))
+ )
+ # [END howto_operator_gcp_create_build_from_yaml_body]
Review comment:
```suggestion
create_build_from_file = CloudBuildCreateOperator(
task_id="create_build_from_file",
project_id=GCP_PROJECT_ID,
body=str(CURRENT_FOLDER.joinpath('example_cloud_build.yaml')),
params={'name': 'Airflow'},
)
# [END howto_operator_gcp_create_build_from_yaml_body]
```
You missed `param` parameter. ;-)
----------------------------------------------------------------
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