Skip to content

#16: Make docker build python setup script#17

Merged
lifflander merged 9 commits intomasterfrom
16-make-docker-build-python-setup-script
Mar 17, 2025
Merged

#16: Make docker build python setup script#17
lifflander merged 9 commits intomasterfrom
16-make-docker-build-python-setup-script

Conversation

@lifflander
Copy link
Contributor

Fixes #16

  • Just start sketching some changes and playing with docker bake

@lifflander lifflander linked an issue Mar 10, 2025 that may be closed by this pull request
Comment on lines +10 to +12
concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Cancel in-progress builds when a new push occurs in a PR or on a branch. We should actually use that in other repos (magistrate?) too.
Azure builds in vt have autoCancel so that's fine:

pr:
  drafts: false
  autoCancel: true

@cz4rs cz4rs linked an issue Mar 12, 2025 that may be closed by this pull request
@cz4rs cz4rs linked an issue Mar 12, 2025 that may be closed by this pull request
@cz4rs cz4rs changed the title #16: workflows: start sketching some changes #16: Make docker build python setup script Mar 12, 2025
Using a repository guarantees consistent state. AFAIK there's no need to download
scripts individually.
@cz4rs cz4rs marked this pull request as ready for review March 13, 2025 16:21
@cz4rs cz4rs requested a review from cwschilly March 13, 2025 16:21
Copy link
Contributor

@cwschilly cwschilly left a comment

Choose a reason for hiding this comment

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

Looks good. Only thing is that some documentation (like how/when to use the docker bake file) might be helpful

@cz4rs
Copy link
Contributor

cz4rs commented Mar 13, 2025

Looks good. Only thing is that some documentation (like how/when to use the docker bake file) might be helpful

Agreed. I'm trying to make it incremental, so bake part is only a scratch at the moment. This PR handles mostly the python setup script.

@cz4rs cz4rs marked this pull request as draft March 14, 2025 09:28
# Clean
RUN rm -rf $WF_TMP_DIR
# Run the setup scripts
RUN --mount=type=bind,rw,source=ci,target=${WF_TMP_DIR} \
Copy link
Contributor

Choose a reason for hiding this comment

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

Mounting as rw to allow saving customized setup script. Docker discards the written data anyways.

https://docs.docker.com/reference/dockerfile/#run---mounttypebind

Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: check if --mount=type=cache can be applied here.

https://docs.docker.com/reference/dockerfile/#run---mounttypecache

@@ -0,0 +1,88 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

More work on this in a follow-up issue.

@cz4rs cz4rs marked this pull request as ready for review March 14, 2025 12:23
@cz4rs cz4rs requested a review from cwschilly March 14, 2025 12:54
@cz4rs
Copy link
Contributor

cz4rs commented Mar 14, 2025

@cwschilly I did some additional clean-up, can you please have a look?
The machinery here will be evolving towards using docker bake, so I'm not adding too many docs. I can improve that once more of the changes from DARMA-tasking/vt#2407 land and maybe #7 is merged too.

Copy link
Contributor Author

@lifflander lifflander left a comment

Choose a reason for hiding this comment

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

Looks good to me, even though I can't approve it since I created it.

Copy link
Contributor

@cwschilly cwschilly left a comment

Choose a reason for hiding this comment

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

LGTM

@lifflander lifflander merged commit d96aad7 into master Mar 17, 2025
27 checks passed
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.

Make docker build python setup script Re-work CI across workflows and VT

3 participants