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

Upstream branch PR: Install packages with Conda in Dockerfile #1509

Merged
merged 12 commits into from
Nov 24, 2023
Merged

Conversation

bquan0
Copy link
Contributor

@bquan0 bquan0 commented Nov 24, 2023

Description

See #1508 for more details on all the changes.
This PR modifies ubuntu_22.04-dev.dockerfile to include separate stages apt_deps and conda-deps to allow for packages to be downloaded by either the package manager apt or conda. It also makes various changes in the workflows docker_publish.yml, build_test.yml, and virtualbox_image.yml to accomodate those changes.

Motivation and Context

This is the same PR as #1508, except that it's made from a branch on the pyne repo instead of my forked repo. This allows the workflows to build the images in the pyne ghcr.io, which will allow us to see if the build_test.yml will pass (since build_test.yml pulls images from the pyne ghcr.io to test).

Behavior

New behavior is that the build_test.yml workflow should run instead of erroring out because it couldn't find the proper image from ghcr.io.

Other Information

I noticed that in virtualbox_image.yml, we never make a VM from the hdf5 images. Is this something I should look into doing?
Also, I was wondering if I should add some logic in build_test.yml to make it not run when the ${{ github.repository_owner }} isn't pyne because the workflow only uses images from the pyne ghcr.io.

Changelog file

All pull requests are required to update the CHANGELOG file with the PR. Your update can take different forms including:

  • creating a new entry describing your change, including a reference to this pull request number
  • adding a reference to your pull request number to an exist entry if that entry can include your changes

@bquan0 bquan0 changed the title Upstream branch PR: Install packages with Conda in Dockerfile. Upstream branch PR: Install packages with Conda in Dockerfile Nov 24, 2023
Copy link
Contributor

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

Thanks for transferring this over @bquan0. I don't think this PR should include the build_test workflow since they CI image won't be available for them to to succeed. That should be a separate PR - thoughts?

@@ -16,6 +16,10 @@ env:
jobs:
pyne_image_build:
runs-on: ubuntu-latest
strategy:
matrix:
pkg_mgr: ['apt', 'conda']
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need both options here. For users relying on the VM, we can pick only one option - apt is probably fine.

@bquan0
Copy link
Contributor Author

bquan0 commented Nov 24, 2023

You're right, I'll revert the changes to build_test.yml and apply them on a separate PR after this one is merged.

Copy link
Contributor

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

Thanks @bquan0 - I look forward to the follow on work here and in conda-forge

@gonuke gonuke merged commit f52b12c into develop Nov 24, 2023
26 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.

2 participants