You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2024/05/10 23:51:49 UTC
[PR] Reapply templates for all providers [airflow]
potiuk opened a new pull request, #39554:
URL: https://github.com/apache/airflow/pull/39554
Follow-up after a few recent changes in provider templates.
The template changes from #39552 have not been applied to the new apache.iceberg provider. Also we are preparing to re-release providers with 2.7.0 compatibility and there was a change in naming of packages updated in the provider doucmentation naming convention that has not been reflected in generated index and fixed in #39341 - but it has not been applied to all the providers.
This PR is a result of `breeze release-management
prepare-provider-documentation --reapply-templates-only` that is supposed to consistently apply template changes to all providers.
<!--
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.
-->
<!--
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] Reapply templates for all providers [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #39554:
URL: https://github.com/apache/airflow/pull/39554#discussion_r1597310053
##########
docs/apache-airflow-providers-tabular/index.rst:
##########
@@ -58,36 +58,63 @@
Detailed list of commits <commits>
-Package apache-airflow-providers-tabular
+apache-airflow-providers-tabular package
------------------------------------------------------
`Tabular <https://tabular.io/>`__
-Release: 1.5.0
+Release: 1.4.1
Review Comment:
1.5.0 is the latest. Probably worth regenerating after we've reverted the docs changes from the iceberg commit.
--
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] Reapply templates for all providers [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on PR #39554:
URL: https://github.com/apache/airflow/pull/39554#issuecomment-2105395204
#39555 fixes tabular, plus some other things that were were broken as well.
#39553 fixes iceberg.
--
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] Reapply templates for all providers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #39554:
URL: https://github.com/apache/airflow/pull/39554
--
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] Reapply templates for all providers [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #39554:
URL: https://github.com/apache/airflow/pull/39554#discussion_r1597369983
##########
airflow/providers/tabular/__init__.py:
##########
@@ -14,3 +14,26 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+#
Review Comment:
This is actually something I asked @Fokko
I wasn't sure if we want to deprecate the whole provider or just the classes in it. Maybe in the future we will have some tabular classes that are not part of the iceberg provider? There is no harm in keeping the provider since we already have 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
Re: [PR] Reapply templates for all providers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39554:
URL: https://github.com/apache/airflow/pull/39554#discussion_r1597372120
##########
airflow/providers/tabular/__init__.py:
##########
@@ -14,3 +14,26 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+#
Review Comment:
I think we can just remove it from the code after we release the new version with iceberg redirection and remove it from the code then. We can always re-add it if there will be a need for tabular-specific 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] Reapply templates for all providers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39554:
URL: https://github.com/apache/airflow/pull/39554#discussion_r1597358000
##########
docs/apache-airflow-providers-tabular/index.rst:
##########
@@ -58,36 +58,63 @@
Detailed list of commits <commits>
-Package apache-airflow-providers-tabular
+apache-airflow-providers-tabular package
------------------------------------------------------
`Tabular <https://tabular.io/>`__
-Release: 1.5.0
+Release: 1.4.1
Review Comment:
Yeah. I think the `__init__.py` removal from tabulrar has hidden some other problems :).
--
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] Reapply templates for all providers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39554:
URL: https://github.com/apache/airflow/pull/39554#issuecomment-2105559818
I think I fixed all issues in this one now.
--
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] Reapply templates for all providers [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on PR #39554:
URL: https://github.com/apache/airflow/pull/39554#issuecomment-2105396600
We could merge this or #39553, doesn't really matter. But we still need the other changes in #39555 either way.
--
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] Reapply templates for all providers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39554:
URL: https://github.com/apache/airflow/pull/39554#discussion_r1597305692
##########
airflow/providers/tabular/__init__.py:
##########
@@ -14,3 +14,26 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+#
Review Comment:
I think that one was removed accidentally - we still want to keep it in `tabular` provider, we want to relase tabular one last time to add `apache.iceberg` as dependency, and we will remove whole `tabular` folder right after.
--
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] Reapply templates for all providers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39554:
URL: https://github.com/apache/airflow/pull/39554#issuecomment-2105390647
cc: @jedcunningham @eladkal @Taragolis -> for the future `breeze release-management prepare-provider-documentation --reapply-templates-only` is there to make sure that changes to templates have been applied consistently everywhere :).
--
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] Reapply templates for all providers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39554:
URL: https://github.com/apache/airflow/pull/39554#discussion_r1597305987
##########
airflow/providers/tabular/__init__.py:
##########
@@ -14,3 +14,26 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+#
Review Comment:
cc: @romsharon98 @eladkal - I guess that was the idea here ? Or was there another reason why wanted to remove the `__init__.py` 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] Reapply templates for all providers [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39554:
URL: https://github.com/apache/airflow/pull/39554#issuecomment-2105561990
We also needed an empty test in `providers/tabular`
--
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