Merging multiple sql operators #9124

samuelkhtu · 2020-06-03T17:44:54Z

Make sure to mark the boxes below before creating PR: [x]

Description above provides context of the change
Unit tests coverage for changes (not needed for documentation changes)
Target Github ISSUE in description if exists
Commits follow "How to write a good git commit message"
Relevant documentation is updated including usage instructions.
I will engage committers as explained in Contribution Workflow Example.

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

1. Merge check_operator.py into airflow.operators.sql 2. Merge sql_branch_operator.py into airflow.operators.sql 3. Merge unit test for both into test_sql.py

samuelkhtu · 2020-06-04T13:19:54Z

When you have a chance, can you take a look? @mik-laj @eladkal. Thanks!

eladkal · 2020-06-04T14:46:17Z

My thoughts (don't take any action for the moment lets wait for thoughts of others)

I'm not sure if it requires deprecation warning for BranchSqlOperator. PR #8525 Add SQL Branch Operator #8942 wasn't released yet. So the question is will it be released with this PR or not. If both PRs are released together then there is no need to maintain airflow/operators/sql_branch_operator.py with deprecation warning.
PrestoCheckOperator(s) were deprecated in Remove Presto check operators #7884 and pointing to CheckOperator(s). By merging this PR they will point to another deprecated module. Maybe worth to fix the deprecation warning for the Presto Operators as well?

samuelkhtu · 2020-06-04T21:36:25Z

Thanks @eladkal , @mik-laj any thought?

mik-laj · 2020-06-09T07:59:38Z

Can you add these cases to the list of changed classes? The file name suggests something different, but that's a different problem. Here we have all the name changes that we then use to generate the documentation.
https://github.com/apache/airflow/blob/master/tests/test_core_to_contrib.py#L427

samuelkhtu · 2020-06-10T21:09:43Z

Yes @mik-laj Thank you for the feedback. I will work on this ASAP.

…l/ValueCheckOperator

airflow/operators/sql_branch_operator.py

tests/test_core_to_contrib.py

samuelkhtu · 2020-06-17T18:26:41Z

Hi @mik-laj when you have a chance, can you take a look? Thanks.

mik-laj · 2020-06-17T18:34:24Z

Close #9099

eladkal · 2020-06-17T19:26:12Z

great work @samuelkhtu
@mik-laj can you cherry pick both PRs #8942 and current to next release? It's core operator so it won't be released in future providers package releases.

mik-laj · 2020-06-17T19:28:02Z

@eladkal I am not a release manager. I focus only on the development of Airflow 2.0

potiuk · 2020-06-17T19:54:53Z

I added it to milestone 1.10.11 it seems easy to cherry pick. Let's see if we manage to do it.

kaxil · 2020-06-17T21:57:56Z

This PR has failing tests

kaxil · 2020-06-17T21:59:20Z

FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_class_deprecated_100_airflow_operators_check_operator_CheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_class_deprecated_101_airflow_operators_check_operator_IntervalCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_class_deprecated_102_airflow_operators_check_operator_ValueCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_class_deprecated_103_airflow_operators_check_operator_ThresholdCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_class_deprecated_104_airflow_operators_sql_branch_operator_BranchSqlOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_subclass_274_airflow_operators_check_operator_CheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_subclass_275_airflow_operators_check_operator_IntervalCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_subclass_276_airflow_operators_check_operator_ValueCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_subclass_277_airflow_operators_check_operator_ThresholdCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_is_subclass_278_airflow_operators_sql_branch_operator_BranchSqlOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_warning_on_import_274_airflow_operators_check_operator_CheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_warning_on_import_275_airflow_operators_check_operator_IntervalCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_warning_on_import_276_airflow_operators_check_operator_ValueCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_warning_on_import_277_airflow_operators_check_operator_ThresholdCheckOperator
FAILED tests/test_core_to_contrib.py::TestMovingCoreToContrib::test_warning_on_import_278_airflow_operators_sql_branch_operator_BranchSqlOperator

mik-laj · 2020-06-17T22:01:43Z

@kaxil I working on it now.

* Merge various SQL Operators into sql.py * Fix unit test code format * Merge multiple SQL operators into one 1. Merge check_operator.py into airflow.operators.sql 2. Merge sql_branch_operator.py into airflow.operators.sql 3. Merge unit test for both into test_sql.py * Rename test_core_to_contrib Interval/ValueCheckOperator to SQLInterval/ValueCheckOperator * Fixed deprecated class and added check to test_core_to_contrib (cherry picked from commit 0b9bf4a)

* Merge various SQL Operators into sql.py * Fix unit test code format * Merge multiple SQL operators into one 1. Merge check_operator.py into airflow.operators.sql 2. Merge sql_branch_operator.py into airflow.operators.sql 3. Merge unit test for both into test_sql.py * Rename test_core_to_contrib Interval/ValueCheckOperator to SQLInterval/ValueCheckOperator * Fixed deprecated class and added check to test_core_to_contrib

* Merge various SQL Operators into sql.py * Fix unit test code format * Merge multiple SQL operators into one 1. Merge check_operator.py into airflow.operators.sql 2. Merge sql_branch_operator.py into airflow.operators.sql 3. Merge unit test for both into test_sql.py * Rename test_core_to_contrib Interval/ValueCheckOperator to SQLInterval/ValueCheckOperator * Fixed deprecated class and added check to test_core_to_contrib (cherry picked from commit 0b9bf4a)

samuelkhtu added 3 commits June 3, 2020 12:34

Merge various SQL Operators into sql.py

5fdfc4b

Fix unit test code format

08fa554

Merge multiple SQL operators into one

d2eb180

1. Merge check_operator.py into airflow.operators.sql 2. Merge sql_branch_operator.py into airflow.operators.sql 3. Merge unit test for both into test_sql.py

boring-cyborg bot added the area:docs label Jun 3, 2020

Rename test_core_to_contrib Interval/ValueCheckOperator to SQLInterva…

669bb37

…l/ValueCheckOperator

mik-laj reviewed Jun 11, 2020

View reviewed changes

airflow/operators/sql_branch_operator.py Outdated Show resolved Hide resolved

mik-laj reviewed Jun 11, 2020

View reviewed changes

tests/test_core_to_contrib.py Outdated Show resolved Hide resolved

Fixed deprecated class and added check to test_core_to_contrib

6f6062f

mik-laj approved these changes Jun 17, 2020

View reviewed changes

mik-laj merged commit 0b9bf4a into apache:master Jun 17, 2020

mik-laj changed the title ~~[AIRFLOW-9099] Merging multiple sql operators~~ Merging multiple sql operators Jun 17, 2020

potiuk added this to the Airflow 1.10.11 milestone Jun 17, 2020

mik-laj pushed a commit to PolideaInternal/airflow that referenced this pull request Jun 17, 2020

Fxi failing from apache#9124

4e4654f

mik-laj mentioned this pull request Jun 17, 2020

Fix failing tests from #9124 #9356

Merged

6 tasks

kaxil pushed a commit that referenced this pull request Jun 17, 2020

Fix failing tests from #9124 (#9356)

281095e

kaxil pushed a commit to kaxil/airflow that referenced this pull request Jun 27, 2020

Fix failing tests from apache#9124 (apache#9356)

fe484e2

eladkal mentioned this pull request Jul 15, 2020

Replace deprecated SQL operators #9844

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merging multiple sql operators #9124

Merging multiple sql operators #9124

samuelkhtu commented Jun 3, 2020

samuelkhtu commented Jun 4, 2020

eladkal commented Jun 4, 2020

samuelkhtu commented Jun 4, 2020

mik-laj commented Jun 9, 2020

samuelkhtu commented Jun 10, 2020

samuelkhtu commented Jun 17, 2020

mik-laj commented Jun 17, 2020

eladkal commented Jun 17, 2020

mik-laj commented Jun 17, 2020

potiuk commented Jun 17, 2020

kaxil commented Jun 17, 2020

kaxil commented Jun 17, 2020

mik-laj commented Jun 17, 2020

Merging multiple sql operators #9124

Merging multiple sql operators #9124

Conversation

samuelkhtu commented Jun 3, 2020

samuelkhtu commented Jun 4, 2020

eladkal commented Jun 4, 2020

samuelkhtu commented Jun 4, 2020

mik-laj commented Jun 9, 2020

samuelkhtu commented Jun 10, 2020

samuelkhtu commented Jun 17, 2020

mik-laj commented Jun 17, 2020

eladkal commented Jun 17, 2020

mik-laj commented Jun 17, 2020

potiuk commented Jun 17, 2020

kaxil commented Jun 17, 2020

kaxil commented Jun 17, 2020

mik-laj commented Jun 17, 2020