Conda Generic Plugin Hooks

[travishathaway]

This is a CEP for implementing a set of generic plugin hooks for conda. Included so far are the following hooks:

• pre_command: allows plugin authors to execute code before a command is run in the conda context
• post_command: allows plugin authors to execute code after a command is run in the conda context

☝️ This list is not final, and we welcome a healthy discussion to debate exactly what belongs here.

[kenodegard]

would it be better to reuse existing terminology (e.g., *_subcommand)?

[travishathaway]

would it be better to reuse existing terminology (e.g., *_subcommand)?

@kenodegard , I'm not exactly sure what that means. Can you please clarify a little?

[kenodegard]

@travishathaway as in use pre_subcommand/post_subcommand instead. It isn't clear what "run" means in pre_run/post_run. Where does it occur? Is it only applicable for the conda run command? Etc.

[travishathaway]

@travishathaway as in use pre_subcommand/post_subcommand instead. It isn't clear what "run" means in pre_run/post_run. Where does it occur? Is it only applicable for the conda run command? Etc.

@kenodegard , I think I'm going to just go with pre_command and post_command to make it just a tad bit shorter to type. It conveys the same meaning.

[jezdez]

Leaving some general naming and conceptual items as feedback since the way this is written at the moment doesn't fully use pluggy's abilties.

Generally speaking it would be useful to pass the actually used command to the pre/post command action, and be a little more detailed about the exception handler implementation since it's a lot different than the pre and post command hooks.

[jezdez]

I think the conda.exceptions.ExceptionHandler should be instead the default exception handler registered (using the tryfirst=True hookimpl parameter)

[travishathaway]

@jezdez, thanks for the review.

When I created the on_exception handler, I was thinking of it more as pseudo event handling system. But, it sounds like what you think may be better is to just allow people to replace or subclass the conda.exceptions.ExceptionHandler class instead.

The "exception_handler" hook would then function just like the "solver" hook. You would only be allowed to configure a single exception handler.

I'm thinking this would be easier from a plugin author's perspective. All they would have to do is the following:

from conda.exceptions import ExceptionHandler

# Example for implementing pretty printing exceptions with "rich"
from rich.console import Console
console = Console()

PLUGN_NAME = "rich"

class RichExceptionHandler(ExceptionHandler):
    """
    Custom exception handler that uses rich to print exceptions.
    """

    def handle_exception(self, exc_val, exc_tb):
        console.print_exception(show_locals=True)

@hookimpl
def conda_exception_handlers(exception):
    """
    Returns our CondaExceptionHandler class which attaches our ``print_exception(``.
    """
    yield CondaExceptionHandler(
        name=f"{PLUGIN_NAME}_exception_handler",
        exception_handler=RichExceptionHandler
    )

We could change the function signature of the handle_exception method on the ExceptionHandler class to pass in the actual exception object too.

What do you think? Is relying on inheritance here a Good Idea™️?

[aovasylenko]

can we review, if current implementation of exception classes should be extended to support custom exception handlers? For example - let's say in the code we have (just found the first one)

    if args.all and prefix == context.default_prefix:
        msg = "cannot remove current environment. deactivate and run conda remove again"
        raise CondaEnvironmentError(msg)

If I want to write a custom exception handler for his exception, I'm missing context - which environment and reason, why it couldn't be removed (or is conda Context the right thing to use?)

If quick question - yes, it worth to review and extend exception handling mechanism - I believe it's another CEP?

[travishathaway]

@aovasylenko, please see @jaimergp's comment below. I think he did a reasonable job of summarizing what you were talking about.

The scenario you are talking about would require a very extensive refactor to existing code, and I do not think it is realistic anytime soon. In the meantime, it's way easier to just rely on the context object like you said.

[jezdez]

@travishathaway Okay, thinking about this a little more, I think we'll have to handle a situation in which multiple plugins implement exception handlers and exist in the runtime environment at the same time.

E.g. imagine if conda-libmamba-solver shipped an exception handler that would enable it to print out additional details of solver errors (e.g. conda/conda-libmamba-solver#102) and the user in addition would like to install a handler that generally renders exceptions with rich (like your example above). The "solver" hook works in that way: there can be multiple solvers installed, and the default setting and the config system determine which to use. In case of exceptions handlers, we probably shouldn't require users to enable them before using them, though. Or is that an indicator to require a plugins enable and plugins disable command to make this an explicit process?

My question is whether we want to enable multiple exception handlers to coexist and create a kind of exception handling pipeline instead of enabling exactly one exception handler?

exception_handlers = plugin_manager.get_hook_results("exception_handlers")
for exception_handler in exception_handlers:
    result = exception_handler.handler(exc_val, exc_tb):
    if result is not None
        return result

The API could require exception handlers to return if they've handled all exceptions, e.g.

class LibmambaExceptionHandler(ExceptionHandler):
    """
    When encountering a libmamba specific error, print it nicely.
    """
    def handle_exception(self, exc_val, exc_tb):
        if isinstance(exc_val, LibMambaUnsatisfiableError):
            return self.handle_libmamba_unsatisfiable_exception(exc_val, exc_tb)

    def handle_libmamba_unsatisfiable_exception(self, exception, traceback):
        ...

@hookimpl
def conda_exception_handlers(exception):
    """
    Returns our CondaExceptionHandler class specific to libmamba exceptions.
    """
    yield CondaExceptionHandler(
        name=f"{PLUGIN_NAME}_exception_handler",
        exception_handler=LibmambaExceptionHandler
    )

@jaimergp You've looked at conda/conda-libmamba-solver#102 a bunch and I think @wolfv has thought about that as well, what's your take on this topic?

[aovasylenko]

will it support run_for as well?

[travishathaway]

I left that out because I didn't think that this plugin should have that fine grain control. It would also complicate the implementation too.

[aovasylenko]

is it optional or mandatory parameter?
(also asked similar question if exception handler hook should have it as well)

[travishathaway]

Good question. I think making this argument optional with the default behavior of it running for every command makes the most sense.

[jaimergp]

A bunch of exceptions in conda are error messages targeting the CLI output for the end user. There's little programmatic intent, and we've often had no other option than to parse error messages back and forth to obtain the required metadata to intercept the exception and handle it.

We could argue that exceptions should be more library friendly, and not so focused on the message but... I actually think it could be seen as a symptom of a bigger problem: exceptions in conda and friends are often used as flow control mechanisms, as an impromptu replacement for more adequate APIs. "I can't solve this? Let's thrown an exception and we'll hope it's caught in the right place!".

Some examples of exceptions-as-flow-control (anti?)pattern:

• Different solver attempts in conda.cli.install and conda.core.solve, leading to this "monstrous invention"
• conda-build logic to find out which packages to build or how to render a recipe
• Disabling conflict analysis in the classic solver changes the retry behaviour because less exceptions are raised in conda.resolve (see unsatisfiable_hints affects whether an environment can be solved conda#9862, Fix unsatisfiable_hints=false bug (misreported conflicts) conda#10867)

So maybe that little mess is something to discuss before or while designing a custom exception handler mechanism? I do agree that having multiple exceptions handlers coexisting seems less limiting than a chain of subclasses. Didn't Python add support for Exception groups or something like that? Is that useful in this case?

[travishathaway]

@jezdez @aovasylenko @jaimergp

I have thought about this some and have decided to take @jaimergp's advice to not include a custom exception handler yet. To use a metaphor, I think we need to clean up the house a bit before inviting guests in 😅. I think that a refactor of the current exception handling mechanism should be done before we revisit this. Once that has been done, we can restart this conversation with a separate CEP.

In the meantime, I will remove exception handling from this CEP and just stick with the pre_command and post_command hooks. These already enable quite a bit.

Once I've removed the exception handling pieces from this CEP, I will move it out of "draft" status. Hopefully, the vote will follow soon afterwards 👍.

[travishathaway]

Example of how our plugin developers could use these hooks:

• Add post env-create hooks conda#10567

[XuehaiPan]

Hi, some questions about the hooks. As I post in the description in PR conda/conda#10567:

• Add post env-create hooks conda#10567

A core usage is to define environment variables when activating/deactivating a conda environment. E.g.:

export CMAKE_PREFIX_PATH="${CONDA_PREFIX}"

NOTE: conda env config vars set does not support variable substitution. So we need hooks here.

In conda/conda#10567, I propose to create a post-create hook to add env-vars.sh to CONDA_PREFIX/etc/activate.d and CONDA_PREFIX/etc/deactivate.d on each environment creation. That approach is covered by this CEP. Now, I can create a CondaPostCommand with run_for={"create"}.

I wonder can the hooks affect the current shell environment like CONDA_PREFIX/etc/activate.d/*.sh do? (not the process of the conda command, which is a subprocess) So I can write post-activate hooks directly rather than etc/activate.d/*.sh. It would be nice to have an example of this in the documentation too.

# post-activate hook
export CONDA_CMAKE_PREFIX_PATH_BACKUP="${CMAKE_PREFIX_PATH}"
export CONDA_LD_LIBRARY_PATH_BACKUP="${LD_LIBRARY_PATH}"
export CMAKE_PREFIX_PATH="${CONDA_PREFIX}"
export LD_LIBRARY_PATH="${CONDA_PREFIX}/lib:${LD_LIBRARY_PATH}"

# pre-deactivate hook
export CMAKE_PREFIX_PATH="${CONDA_CMAKE_PREFIX_PATH_BACKUP}"
export LD_LIBRARY_PATH="${CONDA_LD_LIBRARY_PATH_BACKUP}"
unset CONDA_CMAKE_PREFIX_PATH_BACKUP
unset CONDA_LD_LIBRARY_PATH_BACKUP

[XuehaiPan]

We may also need to document the execution order when multiple hooks are run for the same event:

hook1: run_for={"install", "create"}
hook2: run_for={"install", "update"}

for example, lexicographic order, like ~/.ipython/profile_default/startup/README:

This is the IPython startup directory

.py and .ipy files in this directory will be run *prior* to any code or files specified
via the exec_lines or exec_files configurables whenever you load this profile.

Files will be run in lexicographical order, so you can control the execution order of files
with a prefix, e.g.::

    00-first.py
    50-middle.py
    99-last.ipy

We need to document the order of hook1.pre_command vs. hook2.pre_command and hook1.post_command vs. hook2.post_command. Maybe a FILO stack order?

   hook1.pre_command
-> hook2.pre_command
-> conda command
-> hook2.post_command
-> hook1.post_command

[travishathaway]

@XuehaiPan,

Thanks for your feedback. To answer your question, no this not possible. I researched this myself once and came to the conclusion that simply is not possible because of how shells work (sub-process cannot modify the parent process' environment variable values). This is why we use so much shell specific code in conda and why it's necessary for us to modify a user's shell configuration file (e.g. .zshrc and .bashrc).

[jaimergp]

@XuehaiPan - I think such a plugin would need to add the relevant instructions in (de)activate.d/ directories (or conda-meta/state) as part of its installation or command-line effects. Not that running arbitrary code is a good idea in general, but I get that it's somehow an established practice in the conda ecosystem.