@jaimergp @soapy1 I wonder if this enable_autodetection option is yet another predicate about whether a plugin "can handle", since the same information could simply be implemented on the env spec level instead of the pluggy interface level. I'd prefer the env spec level to all the env spec business logic covered there

I would say that autodetection and the ability for a plugin to parse an environment spec are seperate ideas. Both of these ideas need to come together eventually for conda to decide if an environment spec plugin is going to be used. However, can_handle is about deteriming if a plugin is able to read/parse a given environment spec. And enable_autodetection is about a plugin author being able to opt out of the plugin autodetection process.

imo the whole auto detection process is the responsibility of the plugin manager. So, I think the most appropriate place for a plugin author to be able to set this information is in the bit where the plugin get's registered.

I'm definitely open to suggestions here. But, I would strongly recommend that the mechanism for opting in-to or out-of autodetection is very explicit. Like, it would be confusing if opting out of autodetection was done by having can_handle return a None value opposed to a True/False value.

I don't agree with your hypothesis that plugin authors should be allowed to create plugins that have two states, auto-detectable or not, I'd say those are fundamentally different behaviors, and it's visible by the error message below, which doesn't give users any recourse to resolve the problem.

In other words, I don't agree that this boolean should be stored in the plugin type, since it is business logic if a plugin should be used to automatically detect the environment spec from a provided file. The deciding code would use the same functionality as we'd implement in the EnvironmentSpecBase, rather than what is in the plugin manager, which doesn't care about the implementation details of environment specs at all.

I'd suggest we either move this into the main EnvironmentSpecBase and do something like a detection_supported(), an instance method that can reuse information from the instance:

or we add another plugin type like class OptionalEnvironmentSpec(EnvironmentSpecBase): ..., just for the purpose of providing an optional environment spec, plugin authors would be able to able to switch the behavior by using this different type.

Lets go for adding a detection_supported function opposed to a new type. Pushed up the change.

Why this change? 🤔

good to call this one out! I left a note in the commit message:

Annotating the whole class makes passing refrences to the
class awkward, since it causes 'BinstarSpec' to be a function
opposed to a class. This causes things like CondaEnvironmentSpecifier
to have the wrong type for it's environment_spec field.

So, before this change something like:

for hook in hooks:
        if not hook.environment_spec.detection_supported:
            . . .

would error on the binstar plugin because the environment_spec type is a function instead of a class because it has been decorated.

Moving the deprecation decorator to the __init__ method still produces the result of logging the deprecation whenever the binstar class is instantiated, but avoids the above mentioned awkwardness.

Hm, I'm not sure that's the intended design. cc @kenodegard. We might need an issue to track this if it's not there yet.

Yeah, that's an odd one, I wouldn't block on it TBH, given that the deprecation is ending soon

@kenodegard Do you have a suggestion how to handle this?