Skip to content

fix: register as conda plugin#905

Open
beckermr wants to merge 24 commits intomainfrom
beckermr-patch-1
Open

fix: register as conda plugin#905
beckermr wants to merge 24 commits intomainfrom
beckermr-patch-1

Conversation

@beckermr
Copy link
Copy Markdown

@beckermr beckermr commented Apr 8, 2026

Description

The latest conda versions switched to plugin hooks for custom subcommands (e.g., conda lock) and as of conda 26.3, the old interface was deprecated. This PR adds the proper plugin hooks.

To Do:

  • add plugin hooks
    • add basic code for the hook
    • figure out how to pass argv to click per the todo item
  • adjust logging config so we don't rely on configuring the root logger
  • add tests w/ conda in the python env to ensure the new plugin hooks work as expected

closes #904

Implement conda plugin hooks for conda-lock.
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 8, 2026

Deploy Preview for conda-lock ready!

Name Link
🔨 Latest commit 84a0635
🔍 Latest deploy log https://app.netlify.com/projects/conda-lock/deploys/69d91f3383552d0008676fa7
😎 Deploy Preview https://deploy-preview-905--conda-lock.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Add entry point for conda-lock plugin in pyproject.toml
@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

pre-commit.ci autofix

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

Ah maybe this was never meant to be a subcommand since conda is not listed in the dependencies in the pyproject.toml.

@maresb @mariusvniekerk what is the history on conda lock vs. conda-lock?

@maresb
Copy link
Copy Markdown
Contributor

maresb commented Apr 8, 2026

Hey @beckermr! lock is the default subcommand of conda-lock. I'm not so familiar how conda handles lock as a subcommand, but it seems reasonable that it should work when installed so that conda-lock is equivalent to conda lock. It would probably make sense to allow conda lock conda-lock-subcommand so that for example conda lock install would work. Is this what you mean? I haven't yet looked into your linked issue.

Also, I'm not sure what's up with the CI, but I noticed similar failures last night, so I think it's unrelated to your PR. I'll try to investigate soon.

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

OK. If we merge this PR, then we have to permanently add conda as a dependency of conda-lock. Is that OK?

@maresb
Copy link
Copy Markdown
Contributor

maresb commented Apr 8, 2026

OK. If we merge this PR, then we have to permanently add conda as a dependency of conda-lock. Is that OK?

No, that would break the current pip-installability of conda-lock.

Does the entry-point actually require conda to be a dependency?

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

It currently needs this import from conda.plugins import hookimpl. Maybe I can catch the error and not register? Let me try it.

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

I'll need to add tests here.

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

pre-commit.ci autofix

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

Any hints on the ty test failure @maresb? In principle it requires conda to be installed but that would mean reconfiguring the CI job to not use pypi for the typechecks. I'd rather not do that...

@maresb
Copy link
Copy Markdown
Contributor

maresb commented Apr 8, 2026

Let's use # type: ignore here.

I am a complete n00b at typing so this might be crazy.
@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

LOL that broke mypy 🤦

@maresb
Copy link
Copy Markdown
Contributor

maresb commented Apr 8, 2026

LGTM, I just need to fix the broken CI. Thanks @beckermr!

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

xref: regro/cf-scripts#5889 for the python-build failure

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

This needs integration tests to ensure the conda lock entry point works as expected. I will add those.

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

We need to remove all of the logging.basicConfig calls in order for the plugin to work properly with conda. see conda/conda#15872

@maresb
Copy link
Copy Markdown
Contributor

maresb commented Apr 8, 2026

We need to remove all of the logging.basicConfig calls in order for the plugin to work properly with conda. see conda/conda#15872

Oof, that sounds like a pain. Probably something for a separate PR. Maybe I can get an agent to sort it out.

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

Not too bad. I just did this for smithy. I will push it here.

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

xref: conda-forge/conda-smithy#2512

Added a basic logger configuration function to set up logging.
Co-authored-by: Matthew R. Becker <beckermr@users.noreply.github.com>
@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

pre-commit.ci autofix

@maresb
Copy link
Copy Markdown
Contributor

maresb commented Apr 8, 2026

Looks great @beckermr!

Are you still working on this?

@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

Yes! Let me add a todo list at the top.

@beckermr beckermr marked this pull request as ready for review April 8, 2026 21:58
@beckermr beckermr requested a review from a team as a code owner April 8, 2026 21:59
@beckermr beckermr requested a review from maresb April 8, 2026 21:59
@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 8, 2026

OK @maresb! This one is ready for review!

@beckermr beckermr marked this pull request as draft April 10, 2026 14:33
@beckermr
Copy link
Copy Markdown
Author

beckermr commented Apr 10, 2026

This approach is causing duplicate log messages in smithy. I am going to try a fix there and then update this PR if that works.

@beckermr beckermr marked this pull request as ready for review April 10, 2026 16:16
@beckermr
Copy link
Copy Markdown
Author

ok @maresb - This is ready for review again. The PR is a lot simpler now.

Copy link
Copy Markdown
Contributor

@maresb maresb left a comment

Choose a reason for hiding this comment

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

Sorry it took me a while to review this.

Beautiful simplification with the logging!

My main concern is how this increases the total number of integration test jobs from 18 to 44. I have the impression that some type of API limits cause runner allocation to stall, especially for the macos runners. Thus I'm worried that this increase could lead to major bottlenecks for how long it takes the conda-lock CI to complete.

Would it be sufficient to add a separate job that installs Conda and runs something trivial like conda lock --help to revert the matrix expansion?

Comment on lines +232 to +235
exclude:
- os: windows-latest
python-version: "3.14"
use_conda_cli: "true"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why this exclude? Does setup-miniconda not work yet with python 3.14? Could we add a comment explaining why?

cat conda-lock.yml
mkdir lockfiles
mv conda-$CONDA_PLATFORM.lock conda-$CONDA_PLATFORM.lock.yml conda-lock.yml lockfiles
- name: Set up Python
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Set up Python
- name: Set up Conda

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.

command line integration breaks w/ conda 26.3.0

2 participants