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

Add conda-verify to CI images #237

Closed
wants to merge 1 commit into from

Conversation

jakirkham
Copy link
Member

This adds conda-verify to our CI images to resolve a warning about it being missing.

@jakirkham jakirkham added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Feb 5, 2025
@jakirkham jakirkham requested a review from a team as a code owner February 5, 2025 22:41
@jakirkham jakirkham requested review from gforsyth and removed request for a team February 5, 2025 22:41
@jakirkham
Copy link
Member Author

Think we should wait until 25.02 is out

Canceling CI for now and marking do not merge

We can revisit after the release

@jameslamb
Copy link
Member

Agree with you on not doing this in 25.02.

I have a question in the interim... is installing conda-verify in the base environment enough? I thought from rapidsai/rapids-metadata#43 (comment) that it had to be in the host environment, and that conda-build creates that at build time.

@jakirkham
Copy link
Member Author

Yes conda-verify needs to be installed next to conda-build to be importable

conda-build is in the base environment. We make that explicit here (boa pulled it in before)

@vyasr
Copy link
Contributor

vyasr commented Feb 6, 2025

Is this worth doing in light of rapidsai/build-planning#47? If we migrate everything to rattler-build it's not going to use conda-verify anyway, at which point we might as well remove it to debloat the image no?

@jakirkham
Copy link
Member Author

Which issue/PR is that referencing?

When I click the links it takes me to a 2yr old PR on this repo entitled "WIP: native docker builds". Am guessing that something else was meant or I'm missing context

In any event this came up because conda-verify was coming up in the warnings of a project. So this is an attempt to properly resolve those warnings

I think having the explicit install of conda-build from this PR should go in regardless. We should work to drop boa given it is deprecated and pinning important dependencies to older versions

As to conda-verify, am neutral. It gets rid of a warning and if it doesn't cause problems great. Otherwise it could be skipped

@vyasr
Copy link
Contributor

vyasr commented Feb 6, 2025

Whoops sorry forgot that we were in a different repo, I meant rapidsai/build-planning#47. IOW this feels superfluous given the impending switch to rattler-build.

@jakirkham
Copy link
Member Author

I'll wait to make that determination until we have more light on this question: rapidsai/rapids-metadata#43

@jakirkham
Copy link
Member Author

Following more discussion in the aforementioned issue, we have concluded we can drop conda-verify throughout RAPIDS

This is tracked in issue: rapidsai/build-planning#150

Going to close as won't fix

@jakirkham jakirkham closed this Feb 8, 2025
@jakirkham jakirkham deleted the add_conda-verify branch February 8, 2025 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Hold off on merging; see PR for details improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants