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/11/17 20:27:43 UTC
[GitHub] [airflow] leahecole opened a new pull request #12416: add clarifying language to google backport readme
leahecole opened a new pull request #12416:
URL: https://github.com/apache/airflow/pull/12416
After discovering this the hard way, I wanted to update the docs based on my slack convo with @potiuk and @mik-laj
----------------------------------------------------------------
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] leahecole closed pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole closed pull request #12416:
URL: https://github.com/apache/airflow/pull/12416
----------------------------------------------------------------
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] potiuk commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-763828423
Ah yeah. Sorry for that @leah... It was there waiting for some "freeier" time but it never came :(
----------------------------------------------------------------
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] potiuk commented on a change in pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#discussion_r525872358
##########
File path: airflow/providers/google/ADDITIONAL_INFO.md
##########
@@ -0,0 +1,26 @@
+<!--
+ 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.
+ -->
+
+## Additional limitations
+
+The [2020.11.13 release](#release-20201113) of this provider is only usable with Apache Airflow >= 1.10.12 version due to refactorings implemented in
Review comment:
One comment here @leahecole -> I think it's not precise.
The package, in general, should be usable and installable with 1.10.12, but only the GKEPodOperaotr has some problems with <1.10.12 because cncf.kubernetes cannot be installed there (cncf.kubernetes backport provider is an optional cross-dependency for the google backport provider). So we have actually not limited the google packae to >=1.10.12 - you can still install it with airflow 1.10.9 and it will continue to work (except the GKEPodOperator)
----------------------------------------------------------------
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] leahecole commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-729271257
It seems like it's failing because of lack of license header - I was poking around and it seems like the jinja templates have headers in them, but the generated .md files here do not. Should this new ADDITIONAL_INFO.md comply with the demands of the linter, or should it match with the generated ones?
----------------------------------------------------------------
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] potiuk commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-729221301
The readme is automatically generated based on template: https://github.com/apache/airflow/blob/master/dev/provider_packages/PROVIDER_README_TEMPLATE.md.jinja2. And it will be overwritten the next time we generate it (which is most likely tomorrow).
It's next to impossible to maintain those READMEs separately for 60 providers we have so we always generate them before releasing new set of packages.
However, this template has one place where you can add per-provider specific information that will be added automatically to all the future READMEs - > {{ ADDITIONAL_INFO }}.
It is taken from ADDITIONAL_INFO.md file that should be added in the appropriate provider folder (in this case "airflow/providers/google/ADDITIONAL_INFO.md"). So this explanation should be added there in order to include it next time were generate the README.
----------------------------------------------------------------
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] potiuk commented on a change in pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#discussion_r525872358
##########
File path: airflow/providers/google/ADDITIONAL_INFO.md
##########
@@ -0,0 +1,26 @@
+<!--
+ 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.
+ -->
+
+## Additional limitations
+
+The [2020.11.13 release](#release-20201113) of this provider is only usable with Apache Airflow >= 1.10.12 version due to refactorings implemented in
Review comment:
One comment here @leahecole -> I think it's not precise.
The package, in general, should be usable and installable with <1.10.12 - only the GKEPodOperator has some problems with <1.10.12. This is because cncf.kubernetes provider cannot be installed on airflow < 1.10.11 (and there were some known issues with cncf.kubernetes + 1.10.11). The cncf.kubernetes backport provider is an optional cross-dependency for the google backport provider). So we have actually not limited the google package to >=1.10.12 - you can still install it with airflow 1.10.9 and it will continue to work (except the GKEPodOperator)
----------------------------------------------------------------
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] potiuk commented on a change in pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#discussion_r525875431
##########
File path: airflow/providers/google/ADDITIONAL_INFO.md
##########
@@ -0,0 +1,26 @@
+<!--
+ 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.
+ -->
+
+## Additional limitations
+
+The [2020.11.13 release](#release-20201113) of this provider is only usable with Apache Airflow >= 1.10.12 version due to refactorings implemented in
Review comment:
At least this is what we *think* it is. As far as I remember, you mentioned there are problems with *installing* the provider in composer environment with 1.10.9 so I am wondering where it came from in this case.
----------------------------------------------------------------
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] potiuk edited a comment on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-763828423
Ah yeah. Sorry for that @leahecole It was there waiting for some "freeier" time but it never came :(
----------------------------------------------------------------
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] leahecole commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-729251592
plz enjoy the comic relief of me mistakenly adding and removing blank lines all over the place as evidenced by my commits 😆
----------------------------------------------------------------
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] leahecole commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-729248436
Excellent - makes sense, I'll make that 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-729525169
[The Workflow run](https://github.com/apache/airflow/actions/runs/369740627) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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] leahecole commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-763846841
Ha! No worries. I did it mainly to clean up my PRs because I'm not focused on this right now. As the backport providers mature, I think things will be come more clear and this won't be as much of an 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
[GitHub] [airflow] potiuk commented on a change in pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#discussion_r525872358
##########
File path: airflow/providers/google/ADDITIONAL_INFO.md
##########
@@ -0,0 +1,26 @@
+<!--
+ 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.
+ -->
+
+## Additional limitations
+
+The [2020.11.13 release](#release-20201113) of this provider is only usable with Apache Airflow >= 1.10.12 version due to refactorings implemented in
Review comment:
One comment here @leahecole -> I think it's not precise.
The package, in general, should be usable and installable with <1.10.12, but only the GKEPodOperaotr has some problems with <1.10.12 because cncf.kubernetes cannot be installed there (cncf.kubernetes backport provider is an optional cross-dependency for the google backport provider). So we have actually not limited the google packae to >=1.10.12 - you can still install it with airflow 1.10.9 and it will continue to work (except the GKEPodOperator)
----------------------------------------------------------------
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] potiuk commented on a change in pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#discussion_r525872358
##########
File path: airflow/providers/google/ADDITIONAL_INFO.md
##########
@@ -0,0 +1,26 @@
+<!--
+ 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.
+ -->
+
+## Additional limitations
+
+The [2020.11.13 release](#release-20201113) of this provider is only usable with Apache Airflow >= 1.10.12 version due to refactorings implemented in
Review comment:
One comment here @leahecole -> I think it's not precise.
The package, in general, should be usable and installable with <1.10.12 - only the GKEPodOperator has some problems with <1.10.12. This is because cncf.kubernetes provider cannot be installed on airflow < 1.10.11 (and there were some known issues with cncf.kubernetes + 1.10.11). The cncf.kubernetes backport provider is an optional cross-dependency for the google backport provider). So we have actually not limited the google packae to >=1.10.12 - you can still install it with airflow 1.10.9 and it will continue to work (except the GKEPodOperator)
----------------------------------------------------------------
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] leahecole edited a comment on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole edited a comment on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-729271257
CI seems like it's failing because of lack of license header - I was poking around and it seems like the jinja templates have headers in them, but the generated .md files here do not. Should this new ADDITIONAL_INFO.md comply with the demands of the linter, or should it match with the generated ones?
----------------------------------------------------------------
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] leahecole commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-765001556
I'll take a look at it either later today or tomorrow!
----------------------------------------------------------------
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] leahecole commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-735950372
No prob - I was offline for a few days due to a US holiday. If you want to set up a time to synchronously chat about this communication please let me know, and then if we do that, I can update the comments with our discussion details for transparency :)
----------------------------------------------------------------
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] leahecole commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
leahecole commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-765001556
I'll take a look at it either later today or tomorrow!
----------------------------------------------------------------
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] potiuk commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-729473392
> CI seems like it's failing because of lack of license header - I was poking around and it seems like the jinja templates have headers in them, but the generated .md files here do not. Should this new ADDITIONAL_INFO.md comply with the demands of the linter, or should it match with the generated ones?
It should contain the licence.
The licence comment gets removed when the content is read: https://github.com/apache/airflow/blob/4873d9759dfdec1dd3663074f9e64ad69fa881cc/dev/provider_packages/prepare_provider_packages.py#L1074
And you can see other ADDITIONAL_INFO.md files we have (this is the kubernetes one):
https://raw.githubusercontent.com/apache/airflow/master/airflow/providers/cncf/kubernetes/ADDITIONAL_INFO.md
The rule of thumb is that if a file is generated from other sources, even if it is committed, it does not have to have a licence. It's not per-se documented in ASF rules but this the information that we get when we run the official RAT tool from Apache:
`Generated files do not require license headers`
```
*****************************************************
Summary
-------
Generated at: 2020-11-18T07:51:01+01:00
Notes: 0
Binaries: 0
Archives: 0
Standards: 990
Apache Licensed: 990
Generated Documents: 0
JavaDocs are generated, thus a license header is optional.
Generated files do not require license headers.
0 Unknown Licenses
```
----------------------------------------------------------------
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] potiuk commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-763852211
I think backports are already kinda "on their way out" - they have 2 months EOL (end of March) and we are not going to release them any more after that date. I am more focusing on the regular providers of 2.0:
Maybe you can take a look instead at the new providers docs which we are discussing in https://github.com/apache/airflow/pull/13767#discussion_r560629925 and specifically take a look at the proposal on how the new "provider" documentation will look here: http://gabby-cough.surge.sh/. It would be great to get some comments from someone who might - sooner or later - want to use the providers!
Any comments there are most welcome @leahecole !
----------------------------------------------------------------
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] potiuk commented on pull request #12416: add clarifying language to google backport readme
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12416:
URL: https://github.com/apache/airflow/pull/12416#issuecomment-735481924
Hey @leahecole -> sorry for not getting back on it but we are super-busy with preparing for 2.0.0 (and we have out-of-band 1.10.14 that I am working on getting a consistent set of dependencies). Will come back on it next week!
----------------------------------------------------------------
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