SWAT-59 - ingressClassName support - #703
Conversation
|
LGTM. This introduces the modern cc: @danielhoherd please take a look as well |
| [build-system] | ||
| requires = ["setuptools>=61.0", "wheel"] | ||
| build-backend = "setuptools.build_meta" | ||
|
|
There was a problem hiding this comment.
Why is this needed? We do not build a python package in this repo.
There was a problem hiding this comment.
FWIW the way we use uv with this repo is just to manage the venv, which means build related properties are not needed. EG:
$ uv init
Initialized project `py-example`
$ uv add pytest
Using CPython 3.14.3
Creating virtual environment at: .venv
Resolved 7 packages in 122ms
Installed 5 packages in 17ms
+ iniconfig==2.3.0
+ packaging==26.0
+ pluggy==1.6.0
+ pygments==2.19.2
+ pytest==9.0.2
$ cat pyproject.toml
[project]
name = "py-example"
version = "0.1.0"
description = "Add your description here"
readme = "README.md"
requires-python = ">=3.14"
dependencies = [
"pytest>=9.0.2",
]
There was a problem hiding this comment.
you're right, I didn't use uv locally which caused some issues. with uv everything works as is. I've reverted changes and updated README.md
| echo "Installing uv version ${UV_VERSION}..." | ||
| if [[ -f /tmp/bin/uv ]]; then | ||
| echo "Already installed in /tmp/bin. Skipping!" | ||
| else | ||
| curl --proto '=https' --tlsv1.2 -LsSf https://github.com/astral-sh/uv/releases/download/${UV_VERSION}/uv-installer.sh | sh | ||
| mv "$HOME/.local/bin/uv" /tmp/bin/ | ||
| fi | ||
|
|
There was a problem hiding this comment.
Why is this needed? Was there a problem in CI without this?
danielhoherd
left a comment
danielhoherd
left a comment
There was a problem hiding this comment.
The ingressClass changes look good, but I'd like the uv related changes to be removed.
3 participants
Description
Our helm chart creates ingress configs with deprecated annotations (there's no option to add spec.ingressClassName here.
If global.ingressClassName is set → uses that value
Else if global.extraAnnotations contains kubernetes.io/ingress.class → uses that value
Else → defaults to "{{ .Release.Name }}-nginx"
[build-system]has been added to pyproject.toml to follow the official guidelines here and here along with the build tool.Related Issues
SWAT-59
https://github.com/astronomer/issues/issues/6220
Testing
3 pytests have been added for the configuration updates in the following scenarios:
ingressClassNamewith Airflow onlyingressClassNamewith Airflow and Celery runners (Flower ingress)ingressClassNamewith dag server onlyingressClassNamenot present in the end config when it's not added in valuesMerging
Merge into main