o-nikolas commented on a change in pull request #22311:
URL: https://github.com/apache/airflow/pull/22311#discussion_r829316088



##########
File path: scripts/ci/pre_commit/pre_commit_check_watcher_in_examples.py
##########
@@ -0,0 +1,86 @@
+#!/usr/bin/env python3
+# 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.
+import sys
+from pathlib import Path
+from typing import List
+
+from rich.console import Console
+
+if __name__ not in ("__main__", "__mp_main__"):
+    raise SystemExit(
+        "This file is intended to be executed as an executable program. You 
cannot use it as a module."
+        f"To run this script, run the ./{__file__} command [FILE] ..."
+    )
+
+
+console = Console(color_system="standard", width=200)
+
+errors: List[str] = []
+
+WATCHER_APPEND_INSTRUCTION = "list(dag.tasks) >> watcher()"
+
+PYTEST_FUNCTION = """
+def test_run():

Review comment:
       I see that below this is being copy/pasted into each example dag file. 
What if this function needs to be modified or fixed in the future? Would that 
involve patching every single example dag file? 

##########
File path: tests/system/providers/google/README.md
##########
@@ -0,0 +1,99 @@
+<!--
+ 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.
+-->
+
+# Google provider system tests
+
+## Tests structure
+
+All Google-related system tests are located inside this subdirectory of system 
tests which is
+`tests/system/providers/google/`. They are grouped in directories by the 
related service name, e.g. all BigQuery
+tests are stored inside `tests/system/providers/google/bigquery/` directory. 
In each directory you will find test files
+as self-contained DAGs (one DAG per file). Each test may require some 
additional resources which should be placed in
+`resources` directory found on the same level as tests. Each test file should 
start with prefix `example_*`. If there
+is anything more needed for the test to be executed, it should be documented 
in the docstrings.
+
+Example files structure:
+
+```
+tests/system/providers/google
+├── bigquery
+│   ├── resources
+│   │   ├── example_bigquery_query.sql
+│   │   └── us-states.csv
+│   ├── example_bigquery_queries.py
+│   ├── example_bigquery_operations.py
+.   .
+│   └── example_bigquery_*.py
+├── dataflow
+├── gcs
+.
+└── *
+```
+
+## Initial configuration
+
+Each test requires some environment variables. Check how to set them up on 
your operating system, but on UNIX-based
+OSes this should work:
+
+```commandline
+export NAME_OF_ENV_VAR=value
+```
+
+To confirm that it is set up correctly, run `echo $NAME_OF_ENV_VAR` which will 
display its value.
+
+### Required environment variables
+
+- `SYSTEM_TESTS_GCP_PROJECT` - GCP project name that will be used to run 
system tests (this can be checked on the UI
+  dashboard of the GCP or by running `gcloud config list`).
+
+- `SYSTEM_TESTS_ENV_ID` - environment ID that is unique across between 
different executions of system tests (if they

Review comment:
       Nit:
   
   "`unique across between ...`"
   
   Use either "across" or "between", having both makes the sentence a bit odd.

##########
File path: tests/system/providers/google/bigquery/example_bigquery_dataset.py
##########
@@ -0,0 +1,94 @@
+#
+# 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.
+
+"""
+Example Airflow DAG for Google BigQuery service testing dataset operations.
+"""
+import os
+from datetime import datetime
+
+from airflow import models
+from airflow.operators.bash import BashOperator
+from airflow.providers.google.cloud.operators.bigquery import (
+    BigQueryCreateEmptyDatasetOperator,
+    BigQueryDeleteDatasetOperator,
+    BigQueryGetDatasetOperator,
+    BigQueryUpdateDatasetOperator,
+)
+from airflow.utils.trigger_rule import TriggerRule

Review comment:
       Is this a required component that authors must remember to include for 
system tests to work correctly? If so the pre-commit check should verify this 
is present as well and used below correctly.

##########
File path: scripts/ci/pre_commit/pre_commit_check_watcher_in_examples.py
##########
@@ -0,0 +1,86 @@
+#!/usr/bin/env python3
+# 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.
+import sys
+from pathlib import Path
+from typing import List
+
+from rich.console import Console
+
+if __name__ not in ("__main__", "__mp_main__"):
+    raise SystemExit(
+        "This file is intended to be executed as an executable program. You 
cannot use it as a module."
+        f"To run this script, run the ./{__file__} command [FILE] ..."
+    )
+
+
+console = Console(color_system="standard", width=200)
+
+errors: List[str] = []
+
+WATCHER_APPEND_INSTRUCTION = "list(dag.tasks) >> watcher()"
+
+PYTEST_FUNCTION = """
+def test_run():
+    from airflow.utils.state import State
+
+    dag.clear(dag_run_state=State.NONE)
+    dag.run()
+"""
+
+
+def _check_file(file: Path):
+    content = file.read_text()
+    if "from tests.system.utils.watcher import watcher" in content:

Review comment:
       Someone could import the whole `watcher` module then just use 
`watcher.watcher()` which would evade this check right?




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to