You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "gopidesupavan (via GitHub)" <gi...@apache.org> on 2024/09/16 11:10:02 UTC

[PR] Standard provider bash operator [airflow]

gopidesupavan opened a new pull request, #42252:
URL: https://github.com/apache/airflow/pull/42252

   <!--
    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.
    -->
   Moving bash operators/sensors to standard provider
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   
   
   <!-- Please keep an empty line above the dashes. -->
   ---
   **^ Add meaningful description above**
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2360665234

   moving this bash operator to standard provider, has module path changes in systems test, integration tests.
   
   failures in python client tests, i am thinking due to bashoperator path change, causing dag not loaded.
   Checking remaining 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1774028144


##########
airflow/example_dags/example_bash_operator.py:
##########


Review Comment:
   Okay, yes - I would agree that a refactor of the examples might be possible in a separate PR as well - would not change the scope or structure of the Standard provider.



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2400573370

   > Oh, nooo, merge conflict - can you fix this? then LGTM!
   
   yeah just resolved 3.8 removals ๐Ÿ˜„ 


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1768530640


##########
airflow/providers/standard/provider.yaml:
##########
@@ -42,6 +42,7 @@ operators:
     python-modules:
       - airflow.providers.standard.time.operators.datetime
       - airflow.providers.standard.time.operators.weekday
+      - airflow.providers.standard.core.operators.bash

Review Comment:
   i am ok with suggestion. :) 



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2400563438

   Oh, nooo, merge conflict - can you fix this? then LGTM!


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1767505961


##########
.pre-commit-config.yaml:
##########
@@ -553,7 +553,19 @@ repos:
           ^airflow/example_dags/example_sensors.py|
           ^airflow/example_dags/example_sensors.py|
           ^airflow/example_dags/example_sensors.py|
-          ^airflow/example_dags/example_time_delta_sensor_async.py
+          ^airflow/example_dags/example_time_delta_sensor_async.py|

Review Comment:
   For this change - if we basically add all example DAGs into the exclude list - the check has almost no effect.
   
   Can you re-work the check such that no provider except standard provider dependencies are accepted?



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2365077506

   Additional prod image tests also failing, as it doesnt have the standard provider. inside latest prod image pull.
   <img width="1393" alt="image" src="https://github.com/user-attachments/assets/86f73ade-fe01-49e3-a18b-4bb1e9c0af23">
   


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2365331528

   > After reviewing the logs, it seems the reason the k8s tests are failing is due to the absence of the standard provider package in the image. This is because i guess the standard provider not included in the prod_image_installed_providers.txt file ?
   > 
   > Is it okay to add standard provider in to that file?
   
   Yes. At least from the community discussion we always expected that the new "standard" provider will be a standard install and dependency. So adding to prod image would be the natural consequence. Hope that fixes the problems you had (and sorry for responding late,,,)


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2400903551

   > > Oh, nooo, merge conflict - can you fix this? then LGTM!
   > 
   > yeah just resolved 3.8 removals ๐Ÿ˜„
   
   There is a test failing , but its not related to this. Would you be able to merge please?


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1767519343


##########
.pre-commit-config.yaml:
##########
@@ -553,7 +553,19 @@ repos:
           ^airflow/example_dags/example_sensors.py|
           ^airflow/example_dags/example_sensors.py|
           ^airflow/example_dags/example_sensors.py|
-          ^airflow/example_dags/example_time_delta_sensor_async.py
+          ^airflow/example_dags/example_time_delta_sensor_async.py|

Review Comment:
   Yes, agree. will check.



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1775623716


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -83,7 +83,8 @@ config:
           Exclude some Operators from emitting OpenLineage events by passing a string of semicolon separated
           full import paths of Operators to disable.
         type: string
-        example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
+        example: "airflow.providers.standard.operators.bash.BashOperator;

Review Comment:
   Thanks jens.
   
   Yes small release will help us break the loop, so there's no need to skip the k8s and Docker tests that were recently marked as skipped in the latest commit.
   
   Additionally, hooks related to FileHook and PackageIndexHook are being used in the provider manager, which could also cause test failures when moved to the standard provider.



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1774435228


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -83,7 +83,8 @@ config:
           Exclude some Operators from emitting OpenLineage events by passing a string of semicolon separated
           full import paths of Operators to disable.
         type: string
-        example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
+        example: "airflow.providers.standard.operators.bash.BashOperator;

Review Comment:
   @eladkal This in-deed is a chicken-and-egg problem we had last time with FAB as well. How about if we make a "small" PR such that the standard provoder is ready, release this once (manually) to have it as an empty or half-bake version such that we can un-block this loop?



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2372497637

   The standard provider hasn't been released yet, which is why it's not being installed in the production build images. As a result, k8s and one Docker test are failing. As a temporary, Iโ€™ve updated these tests to be skipped until the standard provider is released and added to prod_image_installed_providers.txt. Could you please let me know if this approach is okay or if you have any other suggestions? :)
   
   I also tried updating prod_image_installed_providers.txt to check if it would install, but itโ€™s still not working.


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1767508246


##########
.pre-commit-config.yaml:
##########
@@ -945,7 +957,10 @@ repos:
         entry: ./scripts/ci/pre_commit/check_system_tests.py
         language: python
         files: ^tests/system/.*/example_[^/]*\.py$
-        exclude: ^tests/system/providers/google/cloud/bigquery/example_bigquery_queries\.py$
+        exclude: >
+          (?x)
+          ^tests/system/providers/google/cloud/bigquery/example_bigquery_queries\.py$ |
+          ^tests/system/providers/standard/time/operators/example_branch_datetime_operator.py

Review Comment:
   Why does this need an exclusion? This is not a system test. Does the check raise a false-positive and need a fix?



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1767510893


##########
airflow/example_dags/example_bash_operator.py:
##########


Review Comment:
   This all looks a bit strange now in my eyes. All example DAGs sitting in the core are requiring a provider package. Smells like a cyclomatic package dependency.
   
   1) Should we move the example DAGs into the provider as well? (Then they are hard to find :-( )
   2) Should we load  the core operators from the compatability alias? (Then we have bad examples)
   3) Any other ideas?



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1774165883


##########
.pre-commit-config.yaml:
##########
@@ -553,7 +553,19 @@ repos:
           ^airflow/example_dags/example_sensors.py|
           ^airflow/example_dags/example_sensors.py|
           ^airflow/example_dags/example_sensors.py|
-          ^airflow/example_dags/example_time_delta_sensor_async.py
+          ^airflow/example_dags/example_time_delta_sensor_async.py|

Review Comment:
   Hope the updated check is fine?



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1770536440


##########
airflow/providers/standard/provider.yaml:
##########
@@ -42,6 +42,7 @@ operators:
     python-modules:
       - airflow.providers.standard.time.operators.datetime
       - airflow.providers.standard.time.operators.weekday
+      - airflow.providers.standard.core.operators.bash

Review Comment:
   removed core/time package folders from standard provider. 



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1770535694


##########
.pre-commit-config.yaml:
##########
@@ -553,7 +553,19 @@ repos:
           ^airflow/example_dags/example_sensors.py|
           ^airflow/example_dags/example_sensors.py|
           ^airflow/example_dags/example_sensors.py|
-          ^airflow/example_dags/example_time_delta_sensor_async.py
+          ^airflow/example_dags/example_time_delta_sensor_async.py|

Review Comment:
   updated to allow only imports from standard provider :)



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1768576493


##########
airflow/example_dags/example_bash_operator.py:
##########


Review Comment:
   What do we do for the other bundled providers - http, sqlite (plus two others - ftp and email)?



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1767995052


##########
airflow/example_dags/example_bash_operator.py:
##########


Review Comment:
   1. Seems reasonable to me. Not that hard to find. But `load_examples` probably needs more work then. Probably not be worth it.
   2. No.
   3. Leave it? Airflow 3 will require the standard provider anyways right?
   
   Side note: We don't need the try/except fallback imo.



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2364640820

   Could you please help on this, kubernetes executor tests are failing with dag not found `example_kubernetes_executor ` and `example_bash_operator`. one option i tried copying the standard package to k8s build container, but its still failing with same reason. 


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2365296145

   After reviewing the logs, it seems the reason the k8s tests are failing is due to the absence of the standard provider package in the image. This is because the standard provider not included in the prod_image_installed_providers.txt file.
   
   Is it okay to add standard provider in to that 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2365183080

   > Additional prod image tests also failing, as it doesnt have the standard provider. inside latest prod image pull. <img alt="image" width="1393" src="https://private-user-images.githubusercontent.com/31437079/369608125-86f73ade-fe01-49e3-a18b-4bb1e9c0af23.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjY5MjQ2MTIsIm5iZiI6MTcyNjkyNDMxMiwicGF0aCI6Ii8zMTQzNzA3OS8zNjk2MDgxMjUtODZmNzNhZGUtZmUwMS00OWUzLWExOGItNGJiMWU5YzBhZjIzLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA5MjElMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwOTIxVDEzMTE1MlomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTNmMGVlODY3YmMzZTc1NzExMmNlOTdjYWMyNGRkMjgyYjRiMDJhNjIwMDViMjg3YTY4YzVjZjU5YzQ0YzhkZmUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.ongG0DAe36JwPInvy74AnBg_OU4gICmsEMd_OisBXks">
   
   Any suggestions for these 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1773352649


##########
airflow/example_dags/example_bash_operator.py:
##########


Review Comment:
   If this refactor is necessary now, I am happy to do part of this or new pr. Please kindly suggest?



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1774168969


##########
airflow/providers/openlineage/provider.yaml:
##########
@@ -83,7 +83,8 @@ config:
           Exclude some Operators from emitting OpenLineage events by passing a string of semicolon separated
           full import paths of Operators to disable.
         type: string
-        example: "airflow.operators.bash.BashOperator;airflow.operators.python.PythonOperator"
+        example: "airflow.providers.standard.operators.bash.BashOperator;

Review Comment:
   I am not sure about this change impact, will it cause any issues? when this provider release in the next provider release cycle, without standard provider? could you please give your thoughts?



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "romsharon98 (via GitHub)" <gi...@apache.org>.
romsharon98 merged PR #42252:
URL: https://github.com/apache/airflow/pull/42252


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1767971878


##########
airflow/example_dags/example_bash_operator.py:
##########


Review Comment:
   Note: This was an open question to the community :-D



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1767996586


##########
airflow/providers/standard/provider.yaml:
##########
@@ -42,6 +42,7 @@ operators:
     python-modules:
       - airflow.providers.standard.time.operators.datetime
       - airflow.providers.standard.time.operators.weekday
+      - airflow.providers.standard.core.operators.bash

Review Comment:
   My 2c, I'm really not sure we need the time/core part of this. Seems better to just have
   
   ```
   airflow.providers.standard.operators.weekday
   airflow.providers.standard.operators.bash
   ```



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2365362544

   > > After reviewing the logs, it seems the reason the k8s tests are failing is due to the absence of the standard provider package in the image. This is because i guess the standard provider not included in the prod_image_installed_providers.txt file ?
   > > Is it okay to add standard provider in to that file?
   > 
   > Yes. At least from the community discussion we always expected that the new "standard" provider will be a standard install and dependency. So adding to prod image would be the natural consequence. Hope that fixes the problems you had (and sorry for responding late,,,)
   
   Thank you :) , I am glad the debug helped me to understand how the ci workflow setups and working process.


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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #42252:
URL: https://github.com/apache/airflow/pull/42252#discussion_r1767528108


##########
.pre-commit-config.yaml:
##########
@@ -945,7 +957,10 @@ repos:
         entry: ./scripts/ci/pre_commit/check_system_tests.py
         language: python
         files: ^tests/system/.*/example_[^/]*\.py$
-        exclude: ^tests/system/providers/google/cloud/bigquery/example_bigquery_queries\.py$
+        exclude: >
+          (?x)
+          ^tests/system/providers/google/cloud/bigquery/example_bigquery_queries\.py$ |
+          ^tests/system/providers/standard/time/operators/example_branch_datetime_operator.py

Review Comment:
   Good catch thank you, have removed that from exclusions.



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


Re: [PR] Standard provider bash operator [airflow]

Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #42252:
URL: https://github.com/apache/airflow/pull/42252#issuecomment-2367396822

   Added standard provider to prod_image_installed_providers, but the CI pipeline is ignoring, might be its not in ready state to consider for constraints while building image? any suggestions.


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