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/08/18 20:02:42 UTC
[GitHub] [airflow] jbmeerkat opened a new pull request, #25802: Added a note about MySQLToS3Operator backward incompatible changes
jbmeerkat opened a new pull request, #25802:
URL: https://github.com/apache/airflow/pull/25802
Changes were made in https://github.com/apache/airflow/pull/20807. As can be seen [in the resulting version of MySQLToS3Operator](https://github.com/mariotaddeucci/airflow/blob/d62f676c89769c455519c085eb79c3799301d798/airflow/providers/amazon/aws/transfers/mysql_to_s3.py) `pd_csv_kwargs` is still accepted but no longer used.
This change silently break things and isn't listed in the release notes. Here's a small discussion about this https://github.com/apache/airflow/commit/bad070f7f484a9b4065a0d86195a1e8002d9bfef#r80597281.
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r949606859
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
Providers are independent from core. you need to look into provider change log as well.
You can upgrade providers regardless of upgrading airflow core
https://github.com/apache/airflow/blob/main/airflow/providers/amazon/CHANGELOG.rst
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r953050476
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
It's explained in depth in https://github.com/apache/airflow#release-process-for-providers
--
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] jbmeerkat commented on a diff in pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
jbmeerkat commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r949584276
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
What file is correct?
--
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 closed pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
URL: https://github.com/apache/airflow/pull/25802
--
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] boring-cyborg[bot] commented on pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #25802:
URL: https://github.com/apache/airflow/pull/25802#issuecomment-1219904099
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r953125046
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
@jbmeerkat - but if you have an idea where to update it, you are absolutely free to do so. You are the best person in the world to make such PR- because a) you just had this problem b) now you know c) you know where you would look for it d) you can write it in the way that other users like you would search for it.
So if you want to make a PR to add such a notice you are most welcome. And it is super easy - just open the right page in the doc and click "Suggest a change on this page" and you will be able to edit it straight in the UI of GitHub. And you are most welcome to do 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.
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r953127263
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
It's eayssy to criticise somethign but in the open source if you see a problem, you contribute a fix rather than complain about it. And I encourage you to do so.
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r953125545
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
This is an easy way to become one of more than 2100 contributors to Airflow. And a good way to pay back for the free software you use.
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r949554683
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
I think also amazon provider 5.0.0 removed it so only the new way will work.
If you believe that the change log of version 3.0.0 is not clear enough then you can add more information to it. PR #20807 was released in version 3.0.0
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r953051685
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
noting also that your assumption is incorrect.
https://airflow.apache.org/docs/apache-airflow/stable/installation/installing-from-pypi.html
You can install Airflow with providers and you can install providers as standalone. It's your choice if you want to couple it or not. Airflow core by itself does not bring provider dependencies.
Quoting from the docs:
`There is also a constraints-no-providers constraint file, which contains just constraints required to install Airflow core. This allows to install and upgrade airflow separately and independently from providers.`
--
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] jbmeerkat commented on a diff in pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
jbmeerkat commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r949577374
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
Thank you for the quick response Elad!
I think it's important to list it here because this change can silently break data. In my case it happened—my custom line separator and column separator were ignored, because `pd_csv_kwargs` is not longer in use, and a lot of data became unusable which resulted in large amount of work.
Honestly speaking I'd consider it a bug. If it's a deprecation nothing should be broken but in this case it was.
--
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] jbmeerkat commented on a diff in pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
jbmeerkat commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r950098232
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
Ok, I'll move the note where you suggest!
Will it be fine to mention somewhere that when upgrading Airflow it's important to take a look into provides' change log? It was not obvious in my case because I just switched from one official image (2.1.0) to another (2.3.2). There were no explicit providers update.
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r949580213
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
It was released in a major version (3.0.0) so if this is a breaking change you can just change the entry in the log from feature to breaking change
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r949579425
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
Sure yeah just add it to the correct 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
[GitHub] [airflow] jbmeerkat commented on a diff in pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
jbmeerkat commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r949582954
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
I don't know yet how it is assumed to be but when I want to upgrade some tool I go and look at its change log or release notes. So when I was upgrading Airflow from 2.1.0 to 2.3.2 I looked into release notes, fixed all the things and still got a huge problem because of the mentioned change. It is not obvious that one should look into each provider change log.
How can I help to prevent things like 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] eladkal commented on a diff in pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r949549607
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
This is not the right place.
The note needs to be in the provider change log not in the Airflow core release notes
--
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 pull request #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25802:
URL: https://github.com/apache/airflow/pull/25802#issuecomment-1228971174
Closing. This is not the right place.
--
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 #25802: Added a note about MySQLToS3Operator backward incompatible changes
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25802:
URL: https://github.com/apache/airflow/pull/25802#discussion_r953051685
##########
RELEASE_NOTES.rst:
##########
@@ -502,6 +502,12 @@ Airflow 2.3.0 restores the original meaning to the parameter. If you set it to "
can be embedded in an iframe (no header is added), but when you set it to "false" the header is added
and Airflow cannot be embedded in an iframe.
+``MySQLToS3Operator`` argument `pd_csv_kwargs` has beed replaced with `pd_kwargs` (#20807)
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Although the old one (``pd_csv_kwargs``) is still accepted but no longer taken into account. Considering
+``MySQLToS3Operator`` deprecation it is recommeded to switch to ``SqlToS3Operator``. Doing this keep in mind
+that ``index`` and ``header`` arguments should be moved into ``pd_kwargs`` dict.
Review Comment:
noting also that your assumption is incorrect.
https://airflow.apache.org/docs/apache-airflow/stable/installation/installing-from-pypi.html
You can install Airflow with providers and you can install providers as standalone. It's your choice if you want to couple it or not. Airflow core by itself does not bring provider dependencies.
--
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