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

ENH: [CI / Docker]: Create a workflow to temporarly build docker images in case dockerfiles are modified #1481

Merged
merged 6 commits into from
Feb 20, 2024

Conversation

younesbelkada
Copy link
Contributor

@younesbelkada younesbelkada commented Feb 19, 2024

Hi !

I would like to enhance the current way of testing docker build workflow, right now I need to manually change the rules for triggering the build docker image job to test if a PR leads to potential failure on docker build. In this PR I propose to simply trigger a simple workflow that just builds (without pushing) docker images that are modified by a PR

cc @pacman100 @BenjaminBossan

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@younesbelkada
Copy link
Contributor Author

@glegendre01 do you think it's ok to move forward with this new workflow?

.github/workflows/test-docker-build.yml Outdated Show resolved Hide resolved
.github/workflows/test-docker-build.yml Outdated Show resolved Hide resolved
Co-authored-by: Guillaume LEGENDRE <glegendre01@gmail.com>
@younesbelkada younesbelkada marked this pull request as ready for review February 19, 2024 09:39
Copy link
Member

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

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

In general, LGTM, thanks for adding this useful feature.

A minor concern I have is about adding this new GH action. Could it be a possible security concern? Maybe it's a non-issue, it was just something that crossed my mind.

@younesbelkada
Copy link
Contributor Author

Thanks for the review!
I think that it should be all good as we don't push any docker image using that workflow and we use a GHA pinned on a specific commit (from @glegendre01 comment), in any case all PRs needs a maintainer approval before running the CI so we should be careful when reviewing PRs where we have some doubts (by pinging @glegendre01 if we have some strong doubts 🙏 )

@younesbelkada
Copy link
Contributor Author

Also I need to merge this PR before properly testing it as GH does not allow to add new workflow from PRs :D

Copy link
Contributor

@pacman100 pacman100 left a comment

Choose a reason for hiding this comment

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

Thank you @younesbelkada, nice QoL update for contributors! 🤗

@younesbelkada younesbelkada merged commit 37dd675 into main Feb 20, 2024
14 checks passed
@younesbelkada younesbelkada deleted the test-workflow branch February 20, 2024 02:55
younesbelkada added a commit that referenced this pull request Mar 5, 2024
* Update test-docker-build.yml

* Update test-docker-build.yml

* Update Dockerfile

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update Dockerfile

* Update .github/workflows/test-docker-build.yml

* Update .github/workflows/test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update .github/workflows/test-docker-build.yml

* Update Dockerfile

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* revert

* Update .github/workflows/test-docker-build.yml

Co-authored-by: Guillaume LEGENDRE <glegendre01@gmail.com>

---------

Co-authored-by: Guillaume LEGENDRE <glegendre01@gmail.com>
BenjaminBossan pushed a commit to BenjaminBossan/peft that referenced this pull request Mar 14, 2024
…images in case dockerfiles are modified (huggingface#1481)

* test workflow

* Update Dockerfile

* build docker images

* Update .github/workflows/test-docker-build.yml

* Update .github/workflows/test-docker-build.yml

Co-authored-by: Guillaume LEGENDRE <glegendre01@gmail.com>

---------

Co-authored-by: Guillaume LEGENDRE <glegendre01@gmail.com>
BenjaminBossan pushed a commit to BenjaminBossan/peft that referenced this pull request Mar 14, 2024
* Update test-docker-build.yml

* Update test-docker-build.yml

* Update Dockerfile

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update Dockerfile

* Update .github/workflows/test-docker-build.yml

* Update .github/workflows/test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update .github/workflows/test-docker-build.yml

* Update Dockerfile

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* Update test-docker-build.yml

* revert

* Update .github/workflows/test-docker-build.yml

Co-authored-by: Guillaume LEGENDRE <glegendre01@gmail.com>

---------

Co-authored-by: Guillaume LEGENDRE <glegendre01@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants