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 2020/06/02 07:32:49 UTC
[GitHub] [airflow] eladkal opened a new issue #9099: moving sql operators into sql.py
eladkal opened a new issue #9099:
URL: https://github.com/apache/airflow/issues/9099
with the addition of BranchSqlOperator https://github.com/apache/airflow/pull/8942 and as pointed during the review I'd like to suggest the following :
```
airflow/operators/sql_branch_operator.py
airflow/operators/check_operator.py
```
will be merged to `airflow/operators/sql.py `
The operators will be renamed as:
CheckOperator -> SqlCheckOperator
ValueCheckOperator -> SqlValueCheckOperator
IntervalCheckOperator -> SqlIntervalCheckOperator
ThresholdCheckOperator ->SqlThresholdCheckOperator
BranchSqlOperator -> the same.
The motivation is that Airflow currently have two separated files in core related to generic SQL.
My suggestion is similar to how PythonOperator(s) are organized in `python.py` (PythonOperator, BranchPythonOperator, ...)
@samuelkhtu you mentioned on slack that you want to take it.
If not I'll add PR for it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #9099:
URL: https://github.com/apache/airflow/issues/9099#issuecomment-637369566
@mik-laj NP. edited
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] samuelkhtu commented on issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
samuelkhtu commented on issue #9099:
URL: https://github.com/apache/airflow/issues/9099#issuecomment-638353877
Hi @eladkal @mik-laj , the PR is ready for review. Thanks.
[https://github.com/apache/airflow/pull/9124](url)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] samuelkhtu commented on issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
samuelkhtu commented on issue #9099:
URL: https://github.com/apache/airflow/issues/9099#issuecomment-637515389
@eladkal Thanks. Can you assign this to me? I can work on 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] samuelkhtu commented on issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
samuelkhtu commented on issue #9099:
URL: https://github.com/apache/airflow/issues/9099#issuecomment-637817437
Should we change the class name from BranchSqlOperator to SQLBranchOperator or use the Python organization? (BranchSQLOperator?)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9099:
URL: https://github.com/apache/airflow/issues/9099#issuecomment-637350362
We should use uppercase for acronyms
> Note: When using acronyms in CapWords, capitalize all the letters of the acronym. Thus HTTPServerError is better than HttpServerError.
https://www.python.org/dev/peps/pep-0008/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj closed issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
mik-laj closed issue #9099:
URL: https://github.com/apache/airflow/issues/9099
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9099:
URL: https://github.com/apache/airflow/issues/9099#issuecomment-637550751
Please start working. I'll do it when I'm on the PC.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] samuelkhtu commented on issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
samuelkhtu commented on issue #9099:
URL: https://github.com/apache/airflow/issues/9099#issuecomment-638186940
Thanks. Sounds good. The PR is done. I will send it out for review later this afternoon.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #9099: moving sql operators into sql.py
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #9099:
URL: https://github.com/apache/airflow/issues/9099#issuecomment-638051733
> Should we change the class name from BranchSqlOperator to SQLBranchOperator or use the Python organization? (BranchSQLOperator?)
It won't be consistent with how it's done in `python.py `:
**Branch**PythonOperator
PythonOperator
PythonVirtualenvOperator
In my opinion the goal here is to make the SQL be consistent with Airflow current convention.
After this step is done you can raise the question of `BranchXOperator` or `XBranchOperator` to discussion as this effect operators from different modules so it's not just SQL local module issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org