Copilot commented on code in PR #69190: URL: https://github.com/apache/airflow/pull/69190#discussion_r3560569795
########## providers/amazon/tests/system/amazon/aws/example_s3_compatible_object_storage.py: ########## @@ -0,0 +1,103 @@ +# +# 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 Dag: input -> transform -> output on an S3-compatible object store via ``ObjectStoragePath``. + +The Amazon provider talks to any S3-compatible object store, so ``ObjectStoragePath("s3://...")`` +reaches the store through the Amazon provider once the ``aws`` connection points at its S3 +endpoint. Amazon S3 is the baseline; the same code works against other S3-compatible services +(for example Amazon S3, Backblaze B2, Cloudflare R2, and MinIO). See the recipe "Use an +S3-compatible object store for Airflow remote task logs" for connection and ``[logging]`` setup. Review Comment: The module docstring says “Amazon S3 is the baseline” and then lists “Amazon S3” again as an example of *other* S3-compatible services. This reads contradictory/redundant; suggest listing only non-AWS examples (or rephrasing to “including Amazon S3 …”). ########## providers/amazon/tests/system/amazon/aws/example_s3_compatible_object_storage.py: ########## @@ -0,0 +1,103 @@ +# +# 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 Dag: input -> transform -> output on an S3-compatible object store via ``ObjectStoragePath``. + +The Amazon provider talks to any S3-compatible object store, so ``ObjectStoragePath("s3://...")`` +reaches the store through the Amazon provider once the ``aws`` connection points at its S3 +endpoint. Amazon S3 is the baseline; the same code works against other S3-compatible services +(for example Amazon S3, Backblaze B2, Cloudflare R2, and MinIO). See the recipe "Use an +S3-compatible object store for Airflow remote task logs" for connection and ``[logging]`` setup. + +Set up an ``aws`` connection (default id ``aws_s3``) whose ``extra`` includes the S3 +``endpoint_url`` and ``region_name``. The bucket name comes from ``S3_BUCKET_NAME``. + +Requires the s3fs extra: ``pip install 'apache-airflow-providers-amazon[s3fs]'``. +""" + +from __future__ import annotations + +import os +from datetime import datetime + +import pytest + +from airflow.sdk import ObjectStoragePath, dag, task + +DAG_ID = "example_s3_compatible_object_storage" + +# Connection id and bucket are read from the environment so the example carries no secrets. +S3_CONN_ID = os.environ.get("S3_CONN_ID", "aws_s3") +S3_BUCKET_NAME_PLACEHOLDER = "replace-with-your-s3-bucket" +S3_BUCKET_NAME = os.environ.get("S3_BUCKET_NAME", S3_BUCKET_NAME_PLACEHOLDER) +BASE_URI = f"s3://{S3_CONN_ID}@{S3_BUCKET_NAME}/airflow-demo/" + + +def _base_path() -> ObjectStoragePath: + if S3_BUCKET_NAME == S3_BUCKET_NAME_PLACEHOLDER: + raise ValueError("Set S3_BUCKET_NAME to a real bucket name before running this Dag.") + return ObjectStoragePath(BASE_URI) + + +@dag( + schedule=None, + start_date=datetime(2021, 1, 1), + catchup=False, + tags=["example", "s3-compatible", "object-storage"], +) Review Comment: `DAG_ID` is defined but not wired into the `@dag` decorator. Setting `dag_id=DAG_ID` makes the dag id explicit (and keeps it stable if the function name changes), or alternatively the constant can be removed. ########## providers/amazon/docs/logging/s3-compatible-remote-logging.rst: ########## @@ -0,0 +1,211 @@ + .. 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. + +.. _write-logs-s3-compatible: + +Use an S3-compatible object store for Airflow remote task logs +============================================================== + +The Amazon provider talks to any S3-compatible object store, not just Amazon S3. Because the +:ref:`S3 remote task handler <write-logs-amazon-s3>` issues standard S3 API calls, pointing the +``aws`` connection at a custom ``endpoint_url`` makes it write Airflow task logs to that +endpoint with no new provider and no core change. You use the ``s3://`` scheme in +``[logging]`` exactly as you would for Amazon S3, and the same connection also backs +``ObjectStoragePath("s3://...")`` for Dag data. + +Amazon S3 is the baseline. The same steps work against other services that expose an +S3-compatible API, for example Amazon S3, Backblaze B2, Cloudflare R2, and MinIO. The only +per-provider differences are the endpoint URL, the region, and whether path-style addressing +is required. Review Comment: This section says “Amazon S3 is the baseline” and then lists “Amazon S3” as an example of *other* S3-compatible services. That’s redundant/contradictory; suggest listing only non-AWS examples (or rephrasing the sentence). -- 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]
