You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Taragolis (via GitHub)" <gi...@apache.org> on 2023/09/18 13:47:01 UTC

[GitHub] [airflow] Taragolis commented on a diff in pull request #34452: Fix system test for GoogleCalendarToGCSOperator

Taragolis commented on code in PR #34452:
URL: https://github.com/apache/airflow/pull/34452#discussion_r1328756870


##########
tests/system/providers/google/cloud/gcs/example_calendar_to_gcs.py:
##########
@@ -44,6 +52,26 @@
         task_id="create_bucket", bucket_name=BUCKET_NAME, project_id=PROJECT_ID
     )
 
+    @task
+    def create_temp_gcp_connection():
+        conn = Connection(
+            conn_id=CONNECTION_ID,
+            conn_type="google_cloud_platform",
+        )
+        conn_extra = {
+            "scope": "https://www.googleapis.com/auth/calendar",
+            "project": PROJECT_ID,
+            "keyfile_dict": "",  # Override to match your needs
+        }
+        conn_extra_json = json.dumps(conn_extra)
+        conn.set_extra(conn_extra_json)
+
+        session: Session = Session()
+        session.add(conn)
+        session.commit()

Review Comment:
   Not related to this PR, we have same pattern in the different system tests.
   
   Just my thought, should we replace connection creation in the future by generic one in System Tests and mark that it only for test purpose (_All the stunts performed by professionals, do not try it yourself at home_ 🤣 ).
   
   Because we use system tests also as example so I wondering that user might decide that this it recommended/supported way to create Connection dynamically
   
   cc @hussein-awala @uranusjr @potiuk @eladkal @o-nikolas 



##########
tests/system/providers/google/cloud/gcs/example_calendar_to_gcs.py:
##########
@@ -44,6 +52,26 @@
         task_id="create_bucket", bucket_name=BUCKET_NAME, project_id=PROJECT_ID
     )
 
+    @task
+    def create_temp_gcp_connection():
+        conn = Connection(
+            conn_id=CONNECTION_ID,
+            conn_type="google_cloud_platform",
+        )
+        conn_extra = {
+            "scope": "https://www.googleapis.com/auth/calendar",
+            "project": PROJECT_ID,
+            "keyfile_dict": "",  # Override to match your needs
+        }
+        conn_extra_json = json.dumps(conn_extra)
+        conn.set_extra(conn_extra_json)
+
+        session: Session = Session()
+        session.add(conn)
+        session.commit()

Review Comment:
   Not related to this PR, we have same pattern in the different system tests.
   
   Just my thought, should we replace connection creation in the future by generic one in System Tests and mark that it only for test purpose (_All the stunts performed by professionals, do not try it yourself at home_ 🤣 ).
   
   Because we use system tests also as example so I wondering that user might decide that this it recommended/supported way to create Connection dynamically
   
   cc @hussein-awala @uranusjr @potiuk @eladkal @o-nikolas 



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org