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 2021/09/10 15:40:02 UTC
[GitHub] [airflow] cablespaghetti opened a new pull request #17211: Chart: Use stable API versions where available
cablespaghetti opened a new pull request #17211:
URL: https://github.com/apache/airflow/pull/17211
Fixes #17188
Uses stable versions of Ingress, PodDisruptionBudget and CronJob APIs where available. There is no change for users of Kubernetes versions older than 1.19. Also tidies up some inconsistencies between the two Ingresses and removes variables on the flower Ingress which didn't do anything.
--
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] kaxil commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-904613152
> New versions of Ingress controllers (eg: `ingress-nginx`) are dropping support for `v1beta1` and Airflow is blocking us from updating to them.
>
> Is there an ETA for this PR?
As soon as @cablespaghetti adds the tests :)
--
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] cablespaghetti commented on a change in pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti commented on a change in pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#discussion_r705338637
##########
File path: chart/values.schema.json
##########
@@ -195,16 +221,6 @@
"default": ""
}
}
- },
- "precedingPaths": {
- "description": "HTTP paths to add to the flower Ingress before the default path.",
- "type": "array",
- "default": []
- },
- "succeedingPaths": {
- "description": "HTTP paths to add to the flower Ingress after the default path.",
- "type": "array",
- "default": []
Review comment:
Done π
--
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] github-actions[bot] commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-887221436
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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] cablespaghetti commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-892473406
> Overall LGTM, thanks @cablespaghetti! Can you add test coverage in `chart/tests` as well though?
Sorry for the delay in this. I will get to this as soon as I can. I was having some trouble getting a working dev environment on my M1 Mac initially but I'm sure it won't be that hard to resolve.
--
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] jedcunningham merged pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
jedcunningham merged pull request #17211:
URL: https://github.com/apache/airflow/pull/17211
--
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] cablespaghetti commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-913200213
I appreciate I can't complain as I took a very long time to complete the work on this PR. However just a reminder to @kaxil and @jedcunningham that this should be ready to go 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
[GitHub] [airflow] jedcunningham commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-921112562
Thanks @cablespaghetti!
--
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] renan commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
renan commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-904575139
New versions of Ingress controllers (eg: `ingress-nginx`) are dropping support for `v1beta1` and Airflow is blocking us from updating to them.
Is there an ETA for this PR?
--
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] cablespaghetti commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-904661269
Really sorry this has taken so long, I have been super busy both at work out and outside. I will put in some time to try and get this done 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham closed pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
jedcunningham closed pull request #17211:
URL: https://github.com/apache/airflow/pull/17211
--
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] jedcunningham commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-917000456
> I don't understand the test failure we've currently got. π
Most likely a flaky failure, let's try it again.
--
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] jedcunningham closed pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
jedcunningham closed pull request #17211:
URL: https://github.com/apache/airflow/pull/17211
--
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] jedcunningham commented on a change in pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#discussion_r704566609
##########
File path: chart/values.schema.json
##########
@@ -195,16 +221,6 @@
"default": ""
}
}
- },
- "precedingPaths": {
- "description": "HTTP paths to add to the flower Ingress before the default path.",
- "type": "array",
- "default": []
- },
- "succeedingPaths": {
- "description": "HTTP paths to add to the flower Ingress after the default path.",
- "type": "array",
- "default": []
Review comment:
Can you add a note to `chart/UPDATING.rst` about these removals?
--
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 #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-886245792
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] cablespaghetti edited a comment on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti edited a comment on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-905692061
Ok hopefully I've done enough now. Also upgraded the default it tests against to 1.22. Rerunning everything locally to check this hasn't broken anything at the moment.
edit: They passed π
--
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 #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-886984991
CC: @jedcunningham @kaxil WDYT ?
--
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] cablespaghetti commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-905692061
Ok hopefully I've done enough now. Also upgraded the default it tests against to 1.22. Rerunning everything locally to check this hasn't broken anything at the moment.
--
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] cablespaghetti commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-916996663
I don't understand the test failure we've currently got. π
--
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] cablespaghetti commented on a change in pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti commented on a change in pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#discussion_r695925270
##########
File path: chart/values.yaml
##########
@@ -120,24 +129,24 @@ ingress:
annotations: {}
# The path for the flower Ingress
- path: ""
+ path: "/"
Review comment:
Both path and pathType are mandatory in the stable api. I don't think setting it by default is likely break many people's config but thought I'd point it out.
--
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] cablespaghetti edited a comment on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti edited a comment on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-904661269
Really sorry this has taken so long, I have been super busy both at work and outside. I will put in some time to try and get this done 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] cablespaghetti commented on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
cablespaghetti commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-916773094
Ok it is quite rightly erroring since I changed the default for Ingress path from "" to "/". I'll change that in the schema and update the UPDATING.rst file. The other error seems to be about an unrelated SQLAlchemy deprecation though so I'll ignore that rather than make an unrelated change in this PR.
--
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 #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-921112371
Awesome work, congrats on your first merged pull request!
--
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] jedcunningham edited a comment on pull request #17211: Chart: Use stable API versions where available
Posted by GitBox <gi...@apache.org>.
jedcunningham edited a comment on pull request #17211:
URL: https://github.com/apache/airflow/pull/17211#issuecomment-921112562
Thanks @cablespaghetti! Congrats on your first 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