Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ci): extract node to own workflow #13059

Merged
merged 4 commits into from
Feb 22, 2023

Conversation

miketheman
Copy link
Member

To simplify the configuration, extract the node-related steps to their own workflow file.

As the node jobs do not require platform dependencies, service containers like postgres or stripe, we can speed up those steps a little by splitting these apart.

To simplify the configuration, extract the node-related steps to their
own workflow file.

As the node jobs do not require platform dependencies, service
containers like postgres or stripe, we can speed up those steps a little
by splitting these apart.

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman
Copy link
Member Author

Timings:

Before, the combined CI workflow "costs" ~26 GitHub Actions minutes

Now with the split, Python ~16 min + Node ~2 min = ~18 minutes, saving 8 minutes of execution time overall

Example runs:
(Python) CI: https://github.com/pypi/warehouse/actions/runs/4246623547/usage
Node CI: https://github.com/pypi/warehouse/actions/runs/4246623544/usage

@miketheman miketheman added the developer experience Anything that improves the experience for Warehouse devs label Feb 22, 2023
@miketheman miketheman marked this pull request as ready for review February 22, 2023 20:27
@miketheman miketheman requested a review from a team as a code owner February 22, 2023 20:27
Copy link
Member

@ewdurbin ewdurbin left a comment

Choose a reason for hiding this comment

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

Nice!

@ewdurbin
Copy link
Member

Doing a think on how (if at all) we need to adjust branch protection before/after merge.

@di
Copy link
Member

di commented Feb 22, 2023

We could merge #12263 which makes it so we don't have to adjust required checks when we add/remove them.

@miketheman
Copy link
Member Author

Doing a think on how (if at all) we need to adjust branch protection before/after merge.

I'm hoping that nothing has to change, since the job names were preserved

@di
Copy link
Member

di commented Feb 22, 2023

I'm hoping that nothing has to change, since the job names were preserved

Aha, yes this does seem to be the case.

@miketheman
Copy link
Member Author

We could merge #12263 which makes it so we don't have to adjust required checks when we add/remove them.

I forgot about that one - I wonder how it works across multiple workflow files?

@ewdurbin
Copy link
Member

We could merge #12263 which makes it so we don't have to adjust required checks when we add/remove them.

I forgot about that one - I wonder how it works across multiple workflow files?

I remember squinting at that and getting confused even before we had multiple workflows.

@miketheman
Copy link
Member Author

hold the phone - I missed something

@miketheman
Copy link
Member Author

miketheman commented Feb 22, 2023

Due to a whoopsie, I had removed the actual test command, restored now.

Numbers are still better - https://github.com/pypi/warehouse/actions/runs/4246789167/usage

Python down to ~21 minutes, so 4 minutes savings, not 8.

@ewdurbin ewdurbin merged commit 31422a2 into pypi:main Feb 22, 2023
@miketheman miketheman deleted the miketheman/gha-refactor branch February 22, 2023 21:48
@webknjaz
Copy link
Member

I forgot about that one - I wonder how it works across multiple workflow files?

@miketheman it is possible to make it work through some refactoring, like in this example PR: pypa/build#618.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer experience Anything that improves the experience for Warehouse devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants