Analyze design of environment management #2288

Activity

1. Eric Snow (ericsnowcurrently)

   added

   feature-requestRequest for new features or functionalityRequest for new features or functionality

   area-terminal

   needs decision

   feature-interpreter

   on Jul 31, 2018
2. ericsnowcurrently commented on Jul 31, 2018

   

   Eric Snow (ericsnowcurrently)

   on Jul 31, 2018

   Author

   More actions

   Also see #1479.
3. brettcannon commented on Jul 31, 2018

   

   Brett Cannon (brettcannon)

   on Jul 31, 2018

   Member

   More actions

   Basically we need to know what kind of shell we use when we run subprocesses to know if we will pick up e.g. environment variables as set by folks in their .bashrc for pyenv (or LC_ALL for Black as another motivating example).
4. Brett Cannon (brettcannon)

   changed the title [-]Improve discovery of virtual environments.[/-] [+]Analyze design of environment discovery/management[/+] on Sep 19, 2018
5. Brett Cannon (brettcannon)

   changed the title [-]Analyze design of environment discovery/management[/-] [+]Analyze design of environment management[/+] on Sep 19, 2018
6. Brett Cannon (brettcannon)

   added this to the 2018, week 38 - September Sprint 2 milestone on Sep 20, 2018
7. DonJayamanne commented on Sep 20, 2018

   

   Don Jayamanne (DonJayamanne)

   on Sep 20, 2018

   Last edited by ericsnowcurrently

   More actions

   Requirements:

   • Ability to discover different types of environments
   • Ability to create an environment for a particular type (create conda, create pip env, create pyenv)
   • Given an interpreter path, be able to determine the environment type for that interpreter
   • Provide the commands necessary to activate an environment for a given shell
     • Why would there be multiple commands?
   • Improved testability
   • Install modules into the environment
   • Discoverability of the code (e.g. all the related code goes in one place)
8. DonJayamanne commented on Sep 20, 2018

   

   Don Jayamanne (DonJayamanne)

   on Sep 20, 2018

   Last edited by DonJayamanne

   More actions

   Ideas

   Note that these overlap substantially. So it isn't just a matter of choosing one. :)

   ---

   Idea 1 (Single class for each type)

   Have a single interface that does everything for a type of Python environment.
   I.e. IEnvironment (concrete implementation for Conda, name CondaEnvironment class):

   • Listing conda environments
   • Activation of conda environments
   • Creation of conda environments
   • Identifying conda environments
     etc.

   IEnvironment:

   • GetInterpreters() : Promise<PythonInterpreter[]>
     Gets a list of interpreters for the type of this implementation (e.g. Conda, Virtua, etc).
   • GetActivationScripts(shell: TerminalShellType): Promise<string[]>
     Gets the activation scripts.
   • GetInterpreterType(pythonPath:string): Promise<InterpretrerType|undefined>
     Gets the type of the interpreter.
   • GetInterpreterType(interpreter: PythonInterpreter): Promise<InterpreterType|undefined>
     Gets the type of the interpreter.
   • CreateEnvironment(name:string, path?:string): Promise
     Creates and environment with a given name and optionally in a specific path (not sure we need the second arg, to be determined).

   Note:

   • Listing environments will return a simple object that defines an environment (with readonly attributes).
   • Much like a repository pattern.

   ---

   Idea 2 (Composition)

   Instead I'm for composition. I.e. IEnvironmentFinder or IEnvironmentProvider, IEnvironmentCreator, IEnvironmentActivator, etc.

   Similar to what we have today, but refactor it to make it more maintainable.
   For instance GlobalWorkspaceLocator might have to be broken up into GlobalPyEnvEnvLocator, GlobalVirtualEnvLocator, CondaLocator, etc...

   Note:

   • Listing environments will return a simple object that defines an environment (with readonly attributes).
   • Much like a repository pattern, but instead of having a single interface to do everything we break it up.
   • I.e. we compose using lightweight interfaces..

   ---

   Idea 3

   Proposal by Derek Keeler (@d3r3kk) that we go with interfaces such as IInterpreter that'll have members such as:

   • Type
   • Path
   • GetVersion()...
   • GetBitness()...
   • GetDisplayName()
   • GetInterpreters()
   • CreateInterpreter()

   This is similar to the first approach but each instance of returning simple data objects we return objects that have can in turn do stuff.

   Derek Keeler (@d3r3kk) please feel free to update accordingly.

   ---

   Idea 4

   Like Option 2, but have one class per environment type that implements the granular interfaces.

   ---

   Idea 5

   3 tiers of info:

   1. distro -- e.g. who provided the Python executable (e.g. python.org, conda, ubuntu)
   2. environment
   3. interpreter

   Here's a rough (and incomplete) sketch the should give you a sense of where I'd put info and functionality.

   export interface IPythonDistribution {
       readonly name: NonEmptyString;
       readonly version: semver.SemVer;
       readonly publisher: NonEmptyString;
       readonly supportedVersions: ISupportedPythonVersions;
       readonly envs: IPythonEnvironments;
   
       /**
        * Add a new virtual environment using the distro.
        */
       createVenv(spec: IVenvSpec, version?: IPythonVersion): IPythonEnvironment;
   }
   
   export interface ISupportedPythonVersions {
       readonly [index: number]: ISupportedPythonVersion;
   }
   
   export interface ISupportedPythonVersion extends IPythonVersion {
       readonly installed: boolean;
   
       /**
        * Ensure that the Python version is installed.
        *
        * If that version of Python is directly usable then a new
        * environment is returned.  Otherwise null is returned.
        */
       ensureInstalled(): IPythonEnvironment | null;
   }
   
   //================================
   // Python environments
   
   export enum PythonEnvKind {
       Unknown,
       Global,
       Venv,
       VirtualEnv,
       PyEnv,
       PipEnv
   }
   
   export interface IPythonEnvironment {
       readonly name: NonEmptyString;
       readonly kind: PythonEnvKind;
       readonly distro: IPythonDistribution;
       readonly interpreter: IPythonInterpreter;
       readonly spec?: IVenvSpec;
       readonly packages: IPythonPackages;
   
       /**
        * Add a new virtual environment based on this env.
        */
       createVenv(IVenvSpec): IPythonEnvironment;
   
       // TODO: activation
   
       /**
        * Remove this environment.
        */
       destroy(): void;
   }
   
   export interface IPythonEnvironments {
       readonly [index: number]: IPythonEnvironment;
   }
   
   export interface IVenvSpec {
       readonly name: NonEmptyString;
       readonly dirname?: NonEmptyString;
   }
   
   //================================
   // Python packages
   
   export interface IPythonPackages {
       lookUp(string | IPythonPackageSpec): ISupportedPythonPackage;
       listAll(): IPythonPackage[];
   }
   
   export interface IPythonPackageSpec {
       readonly name: NonEmptyString;
       readonly version: semver.SemVer;
   }
   
   export interface ISupportedPythonPackage extends IPythonPackageSpec {
       install(): IPythonPackage[];
   }
   
   export interface IPythonPackage extends IPythonPackageSpec {
       uninstall(): void;
   }
   
   //================================
   // Python interpreters
   
   /**
    * A wrapper around a single Python interpreter.
    */
   export interface IPythonInterpreter {
       readonly filename: NonEmptyString;
       readonly version: IPythonVersion;
       readonly env: IPythonEnvironment;
   }
   
   export enum PythonReleaseLevel {
       Alpha,
       Beta,
       Candidate,
       Final
   }
   
   export interface IPythonRelease {
       readonly releaseLevel: PythonReleaseLevel;
       readonly serial: int;
       toString(): string;
   }
   
   export interface IPythonVersion {
       readonly major: int;
       readonly minor: int;
       readonly micro?: int;
       readonly release?: IPythonRelease;
       toString(): string;
   }

   ---

   Idea 6

   • add implementations of the interfaces (e.g. from idea 5) to a global registry
     *add helper functions that pick the right implementation from the registry based on some criteria
   • replace existing environment-specific providers (spread throughout the code base) with generic providers that pull from the global registry for their specific needs (i.e. the providers/services have no knowledge of any specific environment; they are written strictly against the generic interfaces)

   ---

   Idea 7

   <TBD>
9. brettcannon commented on Sep 20, 2018

   

   Brett Cannon (brettcannon)

   on Sep 20, 2018

   Member

   More actions

   I was actually thinking option 1 because as you outlined in the requirements you really only need 4 methods, but I'm not attached to the idea.
10. Don Jayamanne (DonJayamanne)

    self-assigned this

    on Sep 20, 2018
11. DonJayamanne commented on Sep 25, 2018

    

    Don Jayamanne (DonJayamanne)

    on Sep 25, 2018

    More actions

    Derek Keeler (@d3r3kk) Eric Snow (@ericsnowcurrently)
    Please could you review the above options & update if necessary.
12. Eric Snow (ericsnowcurrently)

    assigned

    Derek Keeler (d3r3kk)

    and

    Eric Snow (ericsnowcurrently)

    on Sep 25, 2018
13. DonJayamanne commented on Sep 26, 2018

    

    Don Jayamanne (DonJayamanne)

    on Sep 26, 2018

    More actions

    Here's an incremental approach to solving this issue:

    • Restructure the code to improve discoverability
    • Review code structure and code
    • Start cycle again
14. Brett Cannon (brettcannon)

    closed this as completedon Oct 2, 2018
15. lock

    locked as resolved and limited conversation to collaborators on Oct 30, 2018