AIP-47 - Migrate google marketing DAGs to new design #22447 #24237
Conversation
430a7ba to
945b20c
Compare
|
@bhirsz - I think I'd love your reviews for google ones :) |
| DAG_ID = "example_google_analytics" | ||
| ACCOUNT_ID = os.environ.get("GA_ACCOUNT_ID", "123456789") | ||
|
|
||
| BUCKET = os.environ.get("GMP_ANALYTICS_BUCKET", "test-airflow-analytics-bucket") |
There was a problem hiding this comment.
The migration of the system tests also involves moving the part created in the pytest to the DAG itself. For example in this file:
https://github.com/apache/airflow/pull/24237/files#diff-69116730de9cf0f546bfcebd1c7023fb8342739adb56822af5c73215c0db8d41
You can see that there is bucket created and removed before & after the test. For the migration we also need to do it inside DAG (add create bucket / delete bucket operators). See this PR as example: #23247
| BUCKET = os.environ.get("GMP_DISPLAY_VIDEO_BUCKET", "gs://INVALID BUCKET NAME") | ||
| ADVERTISER_ID = os.environ.get("GMP_ADVERTISER_ID", 1234567) | ||
| OBJECT_NAME = os.environ.get("GMP_OBJECT_NAME", "files/report.csv") | ||
| PATH_TO_UPLOAD_FILE = os.environ.get("GCP_GCS_PATH_TO_UPLOAD_FILE", "test-gcs-example.txt") | ||
| PATH_TO_SAVED_FILE = os.environ.get("GCP_GCS_PATH_TO_SAVED_FILE", "test-gcs-example-download.txt") | ||
| BUCKET_FILE_LOCATION = PATH_TO_UPLOAD_FILE.rpartition("/")[-1] | ||
| SDF_VERSION = os.environ.get("GMP_SDF_VERSION", "SDF_VERSION_5_1") | ||
| BQ_DATA_SET = os.environ.get("GMP_BQ_DATA_SET", "airflow_test") | ||
| GMP_PARTNER_ID = os.environ.get("GMP_PARTNER_ID", 123) | ||
| ENTITY_TYPE = os.environ.get("GMP_ENTITY_TYPE", "LineItem") | ||
| ERF_SOURCE_OBJECT = GoogleDisplayVideo360Hook.erf_uri(GMP_PARTNER_ID, ENTITY_TYPE) |
There was a problem hiding this comment.
The amount of the env variables is one of the problems with the old design. When migrating to the new design it would be best to reduce required env variables rewriting them to constants or fstring reusing ENV_ID and DAG_ID whenever needed. The example of removing the need of environment variable can be seen in this PR: https://github.com/apache/airflow/pull/23282/files#diff-8d21adbd43cba4bdb6175bd7fcc99cc7352a7f2788c9b3486a86148985950a6aL55
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
There was a problem hiding this comment.
In the new design this file is not required. Please ensure that the setup/teardown fixtures are migrated to the DAG itself.
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| import pytest |
There was a problem hiding this comment.
In the new design this file is not required. Please ensure that the setup/teardown fixtures are migrated to the DAG itself.
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| import pytest |
There was a problem hiding this comment.
In the new design this file is not required. Please ensure that the setup/teardown fixtures are migrated to the DAG itself.
| # "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. |
There was a problem hiding this comment.
In the new design this file is not required. Please ensure that the setup/teardown fixtures are migrated to the DAG itself.
Thanks for the mention! And thanks @chethanuk-plutoflume for doing the migration from old to new design. Did you run the system tests in the new design? (most likely not since it would require to have Google Cloud configured). We can cooperate on this front :) I'm currently working on making it possible to run Google system tests by community (in CI). I will let you know when there will be more details so we can try to run this PR using CI. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
|
Needs heavy rebasing :) |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
No description provided.