You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/07/15 18:26:53 UTC

[GitHub] [airflow] eladkal opened a new pull request, #25100: Standardize AwsLambda

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

   The `aws_lambda.py` is not the standard file path. The `aws_` prefix is not needed.
   For the file name I used the same path as the hook.
   
   <!--
   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/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.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+Improvements+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


[GitHub] [airflow] potiuk commented on a diff in pull request #25100: Standardize AwsLambda

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25100:
URL: https://github.com/apache/airflow/pull/25100#discussion_r923508144


##########
airflow/providers/amazon/provider.yaml:
##########
@@ -288,6 +288,7 @@ operators:
   - integration-name: AWS Lambda
     python-modules:
       - airflow.providers.amazon.aws.operators.aws_lambda

Review Comment:
   Hmm. I think we should fix the failing tests actually. I prefer to keep the provider.yaml to describe "current" state not some mixture between and old state - we do not track which entries are deprecated here. 
   
   BTW. Maybe there are other cases like that already but if they are, we should fix it so that the tests do not fail here and right after we should add tests finding similar deprecated entries and remove them. Happy to do it as a follow-up actually.



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


[GitHub] [airflow] potiuk commented on a diff in pull request #25100: Standardize AwsLambda

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25100:
URL: https://github.com/apache/airflow/pull/25100#discussion_r923508144


##########
airflow/providers/amazon/provider.yaml:
##########
@@ -288,6 +288,7 @@ operators:
   - integration-name: AWS Lambda
     python-modules:
       - airflow.providers.amazon.aws.operators.aws_lambda

Review Comment:
   Hmm. I think we should fix the failing tests actually. I prefer to keep the provider.yaml to describe "current" state not some mixture between new and old state - we do not track which entries are deprecated here. 
   
   BTW. Maybe there are other cases like that already but if they are, we should fix it so that the tests do not fail here and right after we should add tests finding similar deprecated entries and remove them. Happy to do it as a follow-up actually.



##########
airflow/providers/amazon/provider.yaml:
##########
@@ -288,6 +288,7 @@ operators:
   - integration-name: AWS Lambda
     python-modules:
       - airflow.providers.amazon.aws.operators.aws_lambda

Review Comment:
   Hmm. I think we should fix the failing tests actually. I prefer to keep the provider.yaml to describe "current" state not some mixture between new and old state - we do not even track which entries are deprecated here. 
   
   BTW. Maybe there are other cases like that already but if they are, we should fix it so that the tests do not fail here and right after we should add tests finding similar deprecated entries and remove them. Happy to do it as a follow-up actually.



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


[GitHub] [airflow] mik-laj commented on a diff in pull request #25100: Standardize AwsLambda

Posted by GitBox <gi...@apache.org>.
mik-laj commented on code in PR #25100:
URL: https://github.com/apache/airflow/pull/25100#discussion_r922639647


##########
airflow/providers/amazon/provider.yaml:
##########
@@ -288,6 +288,7 @@ operators:
   - integration-name: AWS Lambda
     python-modules:
       - airflow.providers.amazon.aws.operators.aws_lambda

Review Comment:
   Is old entry needed? I think, it is only used for documentation. 



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


[GitHub] [airflow] eladkal commented on a diff in pull request #25100: Standardize AwsLambda

Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25100:
URL: https://github.com/apache/airflow/pull/25100#discussion_r922640137


##########
airflow/providers/amazon/provider.yaml:
##########
@@ -288,6 +288,7 @@ operators:
   - integration-name: AWS Lambda
     python-modules:
       - airflow.providers.amazon.aws.operators.aws_lambda

Review Comment:
   Removing it cause test failures.
   We can remove the entry once we remove the operator



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


[GitHub] [airflow] eladkal commented on a diff in pull request #25100: Standardize AwsLambda

Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25100:
URL: https://github.com/apache/airflow/pull/25100#discussion_r924229572


##########
airflow/providers/amazon/provider.yaml:
##########
@@ -288,6 +288,7 @@ operators:
   - integration-name: AWS Lambda
     python-modules:
       - airflow.providers.amazon.aws.operators.aws_lambda

Review Comment:
   I think this better to be handled in a different PR. The provider.yaml is full of such deprecated records



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


[GitHub] [airflow] ferruzzi commented on pull request #25100: Standardize AwsLambda

Posted by GitBox <gi...@apache.org>.
ferruzzi commented on PR #25100:
URL: https://github.com/apache/airflow/pull/25100#issuecomment-1185853831

   Thanks for this.  


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


[GitHub] [airflow] potiuk commented on a diff in pull request #25100: Standardize AwsLambda

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25100:
URL: https://github.com/apache/airflow/pull/25100#discussion_r924336620


##########
airflow/providers/amazon/provider.yaml:
##########
@@ -288,6 +288,7 @@ operators:
   - integration-name: AWS Lambda
     python-modules:
       - airflow.providers.amazon.aws.operators.aws_lambda

Review Comment:
   Really. Haven't checked? That's bad then. Let's do it in separate PR then.



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


[GitHub] [airflow] potiuk merged pull request #25100: Standardize AwsLambda

Posted by GitBox <gi...@apache.org>.
potiuk merged PR #25100:
URL: https://github.com/apache/airflow/pull/25100


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