You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bugraoz93 (via GitHub)" <gi...@apache.org> on 2025/03/22 14:50:55 UTC
[PR] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
bugraoz93 opened a new pull request, #48099:
URL: https://github.com/apache/airflow/pull/48099
Before further implementation, we should have testing in place in the CI. While doing that I realised I also need to update some distribution pieces. Please let me know if I missed anything.
<!--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2746155748
> Looks good, thanks! Just a small nit for a typo.
Thanks for the review!
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2764861052
You will have to rebase some of the changes after https://github.com/apache/airflow/pull/48223 @bugraoz93 -> a numbe of the changes there are somewhat overlapping with this change (mypy changes already include airflow-ctil) - I suggest to merge #48233 , rebase and then I am happy to review/help this one.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019804908
##########
dev/breeze/src/airflow_breeze/utils/packages.py:
##########
@@ -843,6 +843,9 @@ def regenerate_pyproject_toml(
in_optional_dependencies = False
in_additional_devel_dependency_groups = False
for line in pyproject_toml_content.splitlines():
+ print(f"LINE: {line}")
+ if "ctl" in line:
Review Comment:
I was testing something and forgot to delete this. Later, I found where the uv workspaces are generated :) I will remove it. Great one
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2008967468
##########
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##########
@@ -154,7 +154,7 @@ def docker_compose_tests(
sys.exit(return_code)
-TEST_PROGRESS_REGEXP = r"tests/.*|providers/.*/tests/.*|task-sdk/tests/.*|.*=====.*"
+TEST_PROGRESS_REGEXP = r"tests/.*|providers/.*/tests/.*|task-sdk/tests/.*|airflow-ctl/tests/.*|.*=====.*"
Review Comment:
Already merged #48100 :tada:
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2009099120
##########
dev/breeze/src/airflow_breeze/commands/release_management_commands.py:
##########
@@ -611,6 +613,7 @@ def prepare_airflow_distributions(
get_console().print("[success]Successfully prepared Airflow packages")
+# TODO BUGRA CHECK
Review Comment:
Okay, added the command method via again merging non-core distributions other than providers of course.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2009083256
##########
dev/breeze/src/airflow_breeze/commands/release_management_commands.py:
##########
@@ -611,6 +613,7 @@ def prepare_airflow_distributions(
get_console().print("[success]Successfully prepared Airflow packages")
+# TODO BUGRA CHECK
Review Comment:
No, I missed that :facepalm: That's for me and it is for this PR. I need to include that method for airflowctl too because it is built before unit tests in the CI. I will add that method and remove TODO :)
https://github.com/apache/airflow/pull/48099/files#diff-4d1496a96cc8fbdb27e05801df30a444a77bcb80683a2022bc1b41cb23610bdaR88
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2765573918
I dismissed yours @jedcunningham -> your concern was definiely addressed :)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2009006362
##########
dev/breeze/doc/05_test_commands.rst:
##########
@@ -177,6 +177,26 @@ Here is the detailed set of options for the ``breeze testing task-sdk-tests`` co
:alt: Breeze testing task-sdk-tests
+Using ``breeze testing airflow-ctl-tests`` command
+...............................................
+
+The ``breeze testing airflow-ctl-tests`` command is allows you to run tests for Airflow CTL without
+initializing database. The Airflow CTL should not need database to be started so this acts as a
+good check to see if the Airflow CTL tests are working properly.
Review Comment:
```suggestion
The ``breeze testing airflow-ctl-tests`` command allows you to run tests for Airflow CTL without
initializing database. Airflow CTL should not require a database to start, so this acts as a
good check to see if the Airflow CTL tests are working properly.
```
##########
dev/breeze/src/airflow_breeze/commands/release_management_commands.py:
##########
@@ -611,6 +613,7 @@ def prepare_airflow_distributions(
get_console().print("[success]Successfully prepared Airflow packages")
+# TODO BUGRA CHECK
Review Comment:
Is this TODO intended to be left?
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2764922371
Generally, it looks good. Re-reviewed it, and you can even merge it @bugraoz93 -> once @jedcunningham confirms that he is ok . I will take care about resolving conflicts with #48223
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2018722229
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
cc @potiuk if you have any suggestion on how best to handle this limitation
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019883565
##########
Dockerfile.ci:
##########
@@ -589,7 +589,7 @@ function install_airflow() {
# We do not yet use ``uv sync`` because we are not committing and using uv.lock yet and uv sync
# cannot use constraints - once we switch to uv.lock (with the workflow that dependabot will update it
# and constraints will be generated from it, we should be able to simply use ``uv sync`` here
- installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./devel-common --editable ./airflow-core"
+ installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./airflow-ctl --editable ./devel-common"
Review Comment:
Yes, I had hard times once and learned that pre-commit :)
##########
dev/breeze/src/airflow_breeze/utils/packages.py:
##########
@@ -843,6 +843,9 @@ def regenerate_pyproject_toml(
in_optional_dependencies = False
in_additional_devel_dependency_groups = False
for line in pyproject_toml_content.splitlines():
+ print(f"LINE: {line}")
+ if "ctl" in line:
Review Comment:
Removed
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019884059
##########
airflow-ctl/src/airflow/ctl/__main__.py:
##########
@@ -23,7 +23,7 @@
import argcomplete
-from airflowctl.ctl import cli_parser
+from airflow.ctl.cli import cli_parser
Review Comment:
Reverted back to `airflow-ctl/src/airflowctl` :)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2745316241
Seems like we hit the workflow limit :( I will come up with some ideas
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2755341526
I couldn't properly trigger the full-tests even though I have added the label and reopened the issue twice. I will check a bit, please let me know if anyone knows the problem. I will later try to push dummy commits to test the functionality instead. Then I can again squash my commits locally so that they won't be visible on the main.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2764864993
And it looks cool :)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019212973
##########
scripts/ci/testing/run_unit_tests.sh:
##########
@@ -130,13 +130,24 @@ function task_sdk_tests() {
}
+function airflow_ctl_tests() {
+ echo "${COLOR_BLUE}Running Task SDK tests${COLOR_RESET}"
+ set -x
+ breeze testing airflow-ctl-tests
+ set +x
+ echo "${COLOR_BLUE}Task SDK tests completed${COLOR_RESET}"
Review Comment:
```suggestion
echo "${COLOR_BLUE}Airflow CTL tests completed${COLOR_RESET}"
```
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2762240102
LGTM overall, only few nits.
Good work :)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019184804
##########
Dockerfile.ci:
##########
@@ -589,7 +589,7 @@ function install_airflow() {
# We do not yet use ``uv sync`` because we are not committing and using uv.lock yet and uv sync
# cannot use constraints - once we switch to uv.lock (with the workflow that dependabot will update it
# and constraints will be generated from it, we should be able to simply use ``uv sync`` here
- installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./devel-common --editable ./airflow-core"
+ installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./airflow-ctl --editable ./devel-common"
Review Comment:
```suggestion
installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./airflow-ctl --editable ./devel-common"
```
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2764865862
BTW. There should not be many conflicts with #48223
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019803307
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
Thanks @gopidesupavan! Great idea. I agree. This will make things more dynamic so that no need to duplicate steps inside the workflow itself. I will include this change with the other path changes.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2756177530
Okay, fixed that last problem too in a hacky way (static exclude). I added a small method to overcome the problem for now but I will find a way to automate that in the future for non-core distributions that don't need to be in each provider package. Let's not increase the scope, It is ready for review. :)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2759522874
> Okay, fixed that last problem too in a hacky way (static exclude). I added a small method to overcome the problem for now but I will find a way to automate that in the future for non-core distributions that don't need to be in each provider package. Let's not increase the scope, It is ready for review. :)
Removed the hacky way, found the reason and solved 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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2749292583
I closed and reopened to trigger the full tests since it touches those parts. Good to test.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2764865402
Rebased now as there were some ms dependency issues.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 closed pull request #48099: AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands
URL: https://github.com/apache/airflow/pull/48099
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2759683450
```console
#56 writing layer sha256:14dafa0d80636abb67eca9d72fc94ecff7252922e63b16b496435acc6ba87b65 0.0s done
#56 writing layer sha256:16d1ecdd109033d94fdd80d9f9b1f02644626a6d3d1ac2b70df8e3667427e571 0.0s done
#56 writing layer sha256:1b44dfcea9b467af72573b3bd9c591ead7803d87e01e07443fbd6f08e4468d3d 0.1s done
#56 ERROR: error writing layer blob: unexpected status from POST request to https://ghcr.io/v2/apache/airflow/main/ci/python3.9/blobs/uploads/: 403 Forbidden
```
Not related to the changes. My token doesn't work with this CI.
```console
Error: Credentials could not be loaded, please check your action inputs: Could not load credentials from any providers
```
Not related. Seems to be the credential issue from the fork. Since I run canary, it requires more permissions.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2018722229
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
cc @potiuk if you have any suggestion on how best to handle this limitation (or any way to bump that number for us -- if that is possible)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019212471
##########
scripts/ci/testing/run_unit_tests.sh:
##########
@@ -130,13 +130,24 @@ function task_sdk_tests() {
}
+function airflow_ctl_tests() {
+ echo "${COLOR_BLUE}Running Task SDK tests${COLOR_RESET}"
Review Comment:
```suggestion
echo "${COLOR_BLUE}Running Airflow CTL tests${COLOR_RESET}"
```
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2008810826
##########
dev/breeze/doc/09_release_management_tasks.rst:
##########
@@ -689,3 +689,27 @@ default is to build ``both`` type of packages ``sdist`` and ``wheel``.
:target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/doc/images/output_release-management_prepare-task-sdk-distributions.svg
:width: 100%
:alt: Breeze release-management prepare-task-sdk-distributions
+
+
+Preparing airflow ctl packages
Review Comment:
And below (likely there are a few remnants in other places in the docs as well)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2756084109
```
Error: Credentials could not be loaded, please check your action inputs: Could not load credentials from any providers
```
[CI image checks / Publish documentation](https://github.com/apache/airflow/actions/runs/14095702697/job/39483502259?pr=48099#logs) is unrelated.
```
#56 ERROR: error writing layer blob: unexpected status from POST request to https://ghcr.io/v2/apache/airflow/main/ci/python3.9/blobs/uploads/: 403 Forbidden
```
[Additional CI image checks / Push Early Image Cache / Push CI Early:3.9 image cache](https://github.com/apache/airflow/actions/runs/14095702697/job/39482692662?pr=48099#logs) is unrelated.
Static checks seem like going into a circular error where the update provider build hook forces to include the ctl into those pyproject.toml files while SDK generate datamodel hook fails due to that addition. Checking that
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2755591598
Okay, the pre-commit hook forced me to add it as a workspace in all providers. So airflow-ctl seems to need to be defined under as a dev in all providers. Any feedback is welcome
There are still a couple of steps to make it all green :crossed_fingers:
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2008811097
##########
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##########
@@ -154,7 +154,7 @@ def docker_compose_tests(
sys.exit(return_code)
-TEST_PROGRESS_REGEXP = r"tests/.*|providers/.*/tests/.*|task-sdk/tests/.*|.*=====.*"
+TEST_PROGRESS_REGEXP = r"tests/.*|providers/.*/tests/.*|task-sdk/tests/.*|airflow-ctl/tests/.*|.*=====.*"
Review Comment:
```suggestion
TEST_PROGRESS_REGEXP = r"tests/.*|providers/.*/tests/.*|task-sdk/tests/.*|airflow-ctl/tests/.*|.*=====.*"
```
Ach .. I noticed one missed change in my move - here and below. I will fix it separately :)
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2008810741
##########
dev/breeze/doc/09_release_management_tasks.rst:
##########
@@ -689,3 +689,27 @@ default is to build ``both`` type of packages ``sdist`` and ``wheel``.
:target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/doc/images/output_release-management_prepare-task-sdk-distributions.svg
:width: 100%
:alt: Breeze release-management prepare-task-sdk-distributions
+
+
+Preparing airflow ctl packages
Review Comment:
```suggestion
Preparing airflow ctl distributions
```
:D
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 closed pull request #48099: AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands
URL: https://github.com/apache/airflow/pull/48099
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019882986
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
Updated with the suggested approach
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019804908
##########
dev/breeze/src/airflow_breeze/utils/packages.py:
##########
@@ -843,6 +843,9 @@ def regenerate_pyproject_toml(
in_optional_dependencies = False
in_additional_devel_dependency_groups = False
for line in pyproject_toml_content.splitlines():
+ print(f"LINE: {line}")
+ if "ctl" in line:
Review Comment:
I was testing something and forgot to delete this. Later, I found where the uv workspaces are generated :) I will remove it with the print above. Great one
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2008815449
##########
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##########
@@ -154,7 +154,7 @@ def docker_compose_tests(
sys.exit(return_code)
-TEST_PROGRESS_REGEXP = r"tests/.*|providers/.*/tests/.*|task-sdk/tests/.*|.*=====.*"
+TEST_PROGRESS_REGEXP = r"tests/.*|providers/.*/tests/.*|task-sdk/tests/.*|airflow-ctl/tests/.*|.*=====.*"
Review Comment:
My eyes still getting used to :) I see now what you will change :) I can also include it in this one for sure feel free to fix it separately :)
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2745905740
I tried to merge distributions outside core tests in workflows so that the unique number can be persisted and if we have further tests needed for those new distributions, we can add them there without increasing the unique # of workflows. These are just conditional steps in the same workflow. I hope it will be green :crossed_fingers:
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2755391882
> I couldn't properly trigger the full-tests even though I have added the label and reopened the issue twice. I will check a bit, please let me know if anyone knows the problem. I will later try to push dummy commits to test the functionality instead. Then I can again squash my commits locally so that they won't be visible on the main.
Add canary label then it behaves as schedule run :)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019185630
##########
Dockerfile.ci:
##########
@@ -589,7 +589,7 @@ function install_airflow() {
# We do not yet use ``uv sync`` because we are not committing and using uv.lock yet and uv sync
# cannot use constraints - once we switch to uv.lock (with the workflow that dependabot will update it
# and constraints will be generated from it, we should be able to simply use ``uv sync`` here
- installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./devel-common --editable ./airflow-core"
+ installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./airflow-ctl --editable ./devel-common"
Review Comment:
you can update in one of the file, pre-commit will update other 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
Re: [PR] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2020307555
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
> cc @potiuk if you have any suggestion on how best to handle this limitation (or any way to bump that number for us -- if that is possible)
Unfottunately this limitation is hard-coded in GitHub -> https://github.com/orgs/community/discussions/32192
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2765987296
> > Rebased now as there were some ms dependency issues.
>
> Many thanks, @potiuk for your help in this! Also for accepting to rebase from your PR :)
No worries. Already 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
Re: [PR] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019473943
##########
airflow-ctl/src/airflow/ctl/__main__.py:
##########
@@ -23,7 +23,7 @@
import argcomplete
-from airflowctl.ctl import cli_parser
+from airflow.ctl.cli import cli_parser
Review Comment:
Just dropping a "request changes" review here so we don't forget to move this back to `airflowctl`.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019204888
##########
dev/breeze/src/airflow_breeze/utils/packages.py:
##########
@@ -843,6 +843,9 @@ def regenerate_pyproject_toml(
in_optional_dependencies = False
in_additional_devel_dependency_groups = False
for line in pyproject_toml_content.splitlines():
+ print(f"LINE: {line}")
Review Comment:
does this print required? if yes you can use `get_console().print()`
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2765572115
@jedcunningham ?
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #48099:
URL: https://github.com/apache/airflow/pull/48099
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2018555437
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
This is moving Task SDK tests! we still need that 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
Re: [PR] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019213659
##########
scripts/docker/install_airflow.sh:
##########
@@ -51,7 +51,7 @@ function install_airflow() {
# We do not yet use ``uv sync`` because we are not committing and using uv.lock yet and uv sync
# cannot use constraints - once we switch to uv.lock (with the workflow that dependabot will update it
# and constraints will be generated from it, we should be able to simply use ``uv sync`` here
- installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./devel-common --editable ./airflow-core"
+ installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./airflow-ctl --editable ./devel-common"
Review Comment:
```suggestion
installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./airflow-ctl --editable ./devel-common"
```
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019209957
##########
dev/breeze/src/airflow_breeze/utils/packages.py:
##########
@@ -843,6 +843,9 @@ def regenerate_pyproject_toml(
in_optional_dependencies = False
in_additional_devel_dependency_groups = False
for line in pyproject_toml_content.splitlines():
+ print(f"LINE: {line}")
+ if "ctl" in line:
Review Comment:
Not sure this may cause side effect with normal package names?
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2755406538
> > I couldn't properly trigger the full-tests even though I have added the label and reopened the issue twice. I will check a bit, please let me know if anyone knows the problem. I will later try to push dummy commits to test the functionality instead. Then I can again squash my commits locally so that they won't be visible on the main.
>
> Add canary label then it behaves as schedule run :)
That's a good one :) Thanks! I committed a dummy change and just saw your message :sweat_smile:
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2008814982
##########
dev/breeze/doc/09_release_management_tasks.rst:
##########
@@ -689,3 +689,27 @@ default is to build ``both`` type of packages ``sdist`` and ``wheel``.
:target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/doc/images/output_release-management_prepare-task-sdk-distributions.svg
:width: 100%
:alt: Breeze release-management prepare-task-sdk-distributions
+
+
+Preparing airflow ctl packages
Review Comment:
Nice one :D Yes, I will update all of them :)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019181213
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
@bugraoz93 you can do something like this to eliminate if conditons:
Workflow call will be like this
```
with:
distribution-name: "Task SDK"
distribution-cmd-format: "prepare-task-sdk-distributions"
test-type: "task-sdk-tests"
```
Inputs will be like this
```
distribution-name:
description: "The name of the distribution to test"
required: true
type: string
distribution-cmd-format:
description: "The type of distribution to test" #eg prepare-task-sdk-distributions
required: true
type: string
test-type:
description: "distribution test typ" #eg task-sdk-tests
required: true
type: string
```
And finally you pas all the inputs
```
- name: "Prepare Airflow ${{inputs.distribution-name}}: wheel"
env:
DISTRIBUTION_TYPE: "${{ inputs.distribution-cmd-format }}"
run: >
breeze release-management "${DISTRIBUTION_TYPE}" --distribution-format wheel
- name: "Verify wheel packages with twine"
run: |
uv tool uninstall twine || true
uv tool install twine && twine check dist/*.whl
# Conditional steps based on the distribution name
- name: >
Run unit tests for Airflow ${{inputs.distribution-name}}:Python ${{ matrix.python-version }}
env:
PYTHON_VERSION: "${{ matrix.python-version }}"
TEST_TYPE: "${{ inputs.test-type }}"
run: >
breeze testing "${TEST_TYPE}" --python "${PYTHON_VERSION}"
```
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2008967410
##########
dev/breeze/doc/09_release_management_tasks.rst:
##########
@@ -689,3 +689,27 @@ default is to build ``both`` type of packages ``sdist`` and ``wheel``.
:target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/doc/images/output_release-management_prepare-task-sdk-distributions.svg
:width: 100%
:alt: Breeze release-management prepare-task-sdk-distributions
+
+
+Preparing airflow ctl packages
Review Comment:
Updated all packages to distributions in the document
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2020329025
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
nice colution!
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2765869792
> Rebased now as there were some ms dependency issues.
Many thanks, Jarek!
--
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] [WIP] AIP-81 airflowctl Include CI/breeze unit-testing and some distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #48099:
URL: https://github.com/apache/airflow/pull/48099#issuecomment-2745369307
> Nice! Looks good already !
Thanks for the swift review Jarek! I will address them while fixing the workflow limitation.
I will be away from pc for a while, I will come back and make the changes later today :)
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2018710378
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
Thanks for the review, Kaxil! It is there. I first created a separate workflow for airflowctl. Then I hit the limitation of Github. The limitation is to have 20 unique workflows in CI. I merged non-core distribution tests with conditional checks. So single workflow is now running both Task SDK and Airflow CTL tests tasks.
See:
https://github.com/apache/airflow/blob/e79becc882e4424381d44f34013b88f1a355cc8d/.github/workflows/ci.yml#L589-L625
The CI is created with the same workflow. The only difference is that it checks the distribution_name and runs the job steps accordingly. This way, whenever there is be non-core new distribution (other than providers), we can easily add without increasing the # of workflows in the CI.
<img width="921" alt="Screenshot 2025-03-28 at 14 34 25" src="https://github.com/user-attachments/assets/911c8831-c743-4f00-83af-1c367502ff5e" />
Currently, it is impossible to add a new workflow without merging some logic into the CI. :( So what made sense to me is to merge two non-core distributions at the same workflow because their steps are the same for all breeze/CI steps. I don't think we are using the Github workflow file somewhere else. Please correct me if I am missing something
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2018710378
##########
.github/workflows/airflow-distributions-tests.yml:
##########
@@ -16,10 +16,14 @@
# under the License.
Review Comment:
Thanks for the review, Kaxil! It is there. I first created a separate workflow for airflowctl. Then I hit the limitation of Github. The limitation is to have 20 unique workflows in CI. I merged non-core distribution tests with conditional checks. So single workflow is now running both Task SDK and Airflow CTL tests tasks.
See:
https://github.com/apache/airflow/blob/e79becc882e4424381d44f34013b88f1a355cc8d/.github/workflows/ci.yml#L589-L625
The CI is created with the same workflow. The only difference is that it checks the distribution_name and runs the job steps accordingly. This way, whenever there is be non-core new distribution (other than providers), we can easily add without increasing the # of workflows in the CI.
<img width="921" alt="Screenshot 2025-03-28 at 14 34 25" src="https://github.com/user-attachments/assets/911c8831-c743-4f00-83af-1c367502ff5e" />
Currently, it is impossible to add a new workflow without merging some logic into the CI. :( So what made sense to me is to merge two non-core distributions at the same workflow because their steps are the same for all breeze/CI steps. I don't think we are using the Github workflow file somewhere.
--
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] AIP-81 airflowctl Include CI/breeze unit-testing and distribution commands [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #48099:
URL: https://github.com/apache/airflow/pull/48099#discussion_r2019184022
##########
Dockerfile:
##########
@@ -834,7 +834,7 @@ function install_airflow() {
# We do not yet use ``uv sync`` because we are not committing and using uv.lock yet and uv sync
# cannot use constraints - once we switch to uv.lock (with the workflow that dependabot will update it
# and constraints will be generated from it, we should be able to simply use ``uv sync`` here
- installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./devel-common --editable ./airflow-core"
+ installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./airflow-ctl --editable ./devel-common"
Review Comment:
```suggestion
installation_command_flags=" --editable .[${AIRFLOW_EXTRAS}] --editable ./airflow-core --editable ./task-sdk --editable ./airflow-ctl --editable ./devel-common"
```
--
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