Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Apr 23, 2022

Experimentally we've separated some CI-only tools like freespace
and find-newer-dependencies as separate scripts (similarly to
initialize-virtualenv) but contrary to the initialize, our tools
can and should be run in Breeze's virtualenv.

However we are just in the middle of using breeze commands in
the CI context, the commands are not "alien" any more and
they seem to fit the same pattern as other commands.

Since we now have the possibility of grouping sub-commmands in separate
logical groups, it makes perfect sense to bring the tools to the
breeze's sub-commands - mostly because of the auto-complete
support and familiarity of using breeze commands in CI.

This PR brings both freespace and find-newer-dependencies tools
already used in CI to become breeze sub-commands. It also
adds "fix-ownership" command, so that we can run owership
fixing on Linux machines (and adds fixing ownership as
mandatory stop in all breeze-run jobs in CI.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragement file, named {pr_number}.significant.rst, in newsfragments.

@potiuk
Copy link
Member Author

potiuk commented Apr 23, 2022

@edithturn @Bowrna - seems that with the recent changes where I convert most of the "ci" to use the new "breeze" commands - moving freespace and my other command find-newer-dependencies as breeze sub-command started to make sense so I bring it back now.

@potiuk potiuk force-pushed the unify-ci-tools branch 7 times, most recently from 1ef8ec6 to 53896f1 Compare April 24, 2022 08:23
Experimentally we've separated some CI-only tools like freespace
and find-newer-dependencies as separate scripts (similarly to
initialize-virtualenv) but contrary to the initialize, our tools
can and should be run in Breeze's virtualenv.

However we are just in the middle of using breeze commands in
the CI context, the commands are not "alien" any more and
they seem to fit the same pattern as other commands.

Since we now have the possibility of grouping sub-commmands in separate
logical groups, it makes perfect sense to bring the tools to the
breeze's sub-commands - mostly because of the auto-complete
support and familiarity of using breeze commands in CI.

This PR brings both freespace and find-newer-dependencies tools
already used in CI to become breeze sub-commands. It also
adds "fix-ownership" and "resource-check" commands, so that we can run
owership fixing on Linux machines (and adds fixing ownership as
mandatory stop in all breeze-run jobs in CI and that users
can run resource-check locally.

This is because thgere were some cases after introducing Breeze that
some root-owned left-overs prevented to checkout code by the
runner on self-hosted runners.

This was likely due to resource-check that was run without
PYTHONDONTWRITEBYTECODE=1 variable - this variable have been
added now to execution of the python script.
* Checking available resources for docker with ``breeze resource-check`` command
* Freeing space needed to run CI tests with ``breeze free-space`` command
* Fixing ownership of files in your repository with ``breeze fix-ownership`` command
* Print Breeze version with ``breeze fix-ownership`` command
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Print Breeze version with ``breeze fix-ownership`` command
* Print Breeze version with ``breeze version`` command

Should it be breeze version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah :)

@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Apr 24, 2022
@potiuk potiuk merged commit 87e3733 into apache:main Apr 24, 2022
@potiuk potiuk deleted the unify-ci-tools branch April 24, 2022 15:33
potiuk added a commit to potiuk/airflow that referenced this pull request Apr 24, 2022
Static checks were not really "enabled' after migration since apache#23193

This PR fixes it.
potiuk added a commit that referenced this pull request Apr 24, 2022
Static checks were not really "enabled' after migration since #23193

This PR fixes it.
@jedcunningham jedcunningham added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Apr 25, 2022
@potiuk potiuk restored the unify-ci-tools branch April 26, 2022 20:49
@potiuk potiuk deleted the unify-ci-tools branch July 29, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants