Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions crates/pet-conda/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,9 @@ pet-utils = { path = "../pet-utils" }
log = "0.4.21"
regex = "1.10.4"

[dev-dependencies]
pet-reporter = { path = "../pet-reporter" }


[features]
ci = []
140 changes: 77 additions & 63 deletions crates/pet-conda/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use environments::{get_conda_environment_info, CondaEnvironment};
use log::error;
use manager::CondaManager;
use pet_core::{
os_environment::Environment, python_environment::PythonEnvironment, Locator, LocatorResult,
os_environment::Environment, python_environment::PythonEnvironment, reporter::Reporter,
Locator, LocatorResult,
};
use pet_utils::env::PythonEnv;
use std::{
Expand Down Expand Up @@ -147,73 +148,86 @@ impl Locator for Conda {
None
}

fn find(&self) -> Option<LocatorResult> {
// 1. Get a list of all know conda environments
let known_conda_envs =
get_conda_environments(&get_conda_environment_paths(&self.env_vars), &None);
let mut new_managers = vec![];
{
let mut managers = self.managers.lock().unwrap();
// 2. Go through all conda dirs and build the conda managers.
for env in &known_conda_envs {
if let Some(conda_dir) = &env.conda_dir {
if managers.contains_key(conda_dir) {
continue;
fn find(&self, reporter: &dyn Reporter) {
let env_vars = self.env_vars.clone();
thread::scope(|s| {
// 1. Get a list of all know conda environments file paths
let possible_conda_envs = get_conda_environment_paths(&env_vars);
for path in possible_conda_envs {
s.spawn(move || {
// 2. Get the details of the conda environment
// This we do not get any details, then its not a conda environment
let env = get_conda_environment_info(&path, &None)?;

// 3. If we have a conda environment without a conda_dir
// Then we will not be able to get the manager.
// Either way report this environment
if env.conda_dir.is_none(){
// We will still return the conda env even though we do not have the manager.
// This might seem incorrect, however the tool is about discovering environments.
// The client can activate this env either using another conda manager or using the activation scripts
error!("Unable to find Conda Manager for the Conda env: {:?}", env);
let prefix = env.prefix.clone();
let env = env.to_python_environment(None, None);
let mut environments = self.environments.lock().unwrap();
environments.insert(prefix, env.clone());
reporter.report_environment(&env);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as soon as we discover anything we report it.
Previously we-d collect and return to calling code.

return None;
}
if let Some(manager) = CondaManager::from(conda_dir) {
new_managers.push(manager.to_manager());
managers.insert(conda_dir.clone(), manager);

// 3. We have a conda environment with a conda_dir (above we handled the case when its not found)
// We will try to get the manager for this conda_dir
let prefix = env.clone().prefix.clone();

{
// 3.1 Check if we have already reported this environment.
// Closure to quickly release lock
let environments = self.environments.lock().unwrap();
if environments.contains_key(&env.prefix) {
return None;
}
}
}
}
}

let mut environments = self.environments.lock().unwrap();
let mut new_environments: Vec<PythonEnvironment> = vec![];
// 3. Go through each environment we know of and build the python environments.
for known_env in &known_conda_envs {
if environments.contains_key(&known_env.prefix) {
continue;
}
if let Some(conda_dir) = &known_env.conda_dir {
if let Some(manager) = self.get_manager(conda_dir) {
let env = known_env.to_python_environment(
Some(manager.conda_dir.clone()),
Some(manager.to_manager()),
);
environments.insert(known_env.prefix.clone(), env.clone());
new_environments.push(env);
} else {
// We will still return the conda env even though we do not have the manager.
// This might seem incorrect, however the tool is about discovering environments.
// The client can activate this env either using another conda manager or using the activation scripts
error!("Unable to find Conda Manager for Conda env (even though we have a conda_dir {:?}): Env Details = {:?}", conda_dir, known_env);
let env = known_env.to_python_environment(Some(conda_dir.clone()), None);
environments.insert(known_env.prefix.clone(), env.clone());
new_environments.push(env);
}
} else {
// We will still return the conda env even though we do not have the manager.
// This might seem incorrect, however the tool is about discovering environments.
// The client can activate this env either using another conda manager or using the activation scripts
error!(
"Unable to find Conda Manager for the Conda env: {:?}",
known_env
);
let env = known_env.to_python_environment(None, None);
environments.insert(known_env.prefix.clone(), env.clone());
new_environments.push(env);
}
}

if new_managers.is_empty() && new_environments.is_empty() {
return None;
}
// 4 Get the manager for this env.
let conda_dir = &env.conda_dir.clone()?;
let managers = self.managers.lock().unwrap();
let mut manager = managers.get(conda_dir).cloned();
drop(managers);

Some(LocatorResult {
managers: new_managers,
environments: new_environments,
})
if manager.is_none() {
// 4.1 Build the manager from the conda dir if we do not have it.
if let Some(conda_manager) = CondaManager::from(conda_dir) {
reporter.report_manager(&conda_manager.to_manager());
let mut managers = self.managers.lock().unwrap();
managers.insert(conda_dir.to_path_buf().clone(), conda_manager.clone());
manager = Some(conda_manager);
}
}

// 5. Report this env.
if let Some(manager) = manager {
let env = env.to_python_environment(
Some(manager.conda_dir.clone()),
Some(manager.to_manager()),
);
let mut environments = self.environments.lock().unwrap();
environments.insert(prefix.clone(), env.clone());
reporter.report_environment(&env);
} else {
// We will still return the conda env even though we do not have the manager.
// This might seem incorrect, however the tool is about discovering environments.
// The client can activate this env either using another conda manager or using the activation scripts
error!("Unable to find Conda Manager for Conda env (even though we have a conda_dir {:?}): Env Details = {:?}", conda_dir, env);
let env = env.to_python_environment(Some(conda_dir.clone()), None);
let mut environments = self.environments.lock().unwrap();
environments.insert(prefix.clone(), env.clone());
reporter.report_environment(&env);
}
Option::<()>::Some(())
});
}
});
}
}

Expand Down
26 changes: 20 additions & 6 deletions crates/pet-conda/tests/ci_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ fn detect_conda_root() {
manager::EnvManagerType, os_environment::EnvironmentApi,
python_environment::PythonEnvironmentCategory, Locator,
};
use std::path::PathBuf;
use pet_reporter::test::create_reporter;

let env = EnvironmentApi::new();

let reporter = create_reporter();
let conda = Conda::from(&env);
let result = conda.find().unwrap();
conda.find(&reporter);
let result = reporter.get_result();

assert_eq!(result.managers.len(), 1);

Expand Down Expand Up @@ -92,6 +94,7 @@ fn detect_new_conda_env() {
use pet_core::{
os_environment::EnvironmentApi, python_environment::PythonEnvironmentCategory, Locator,
};
use pet_reporter::test::create_reporter;
use std::path::PathBuf;

let env_name = "env_with_python";
Expand All @@ -102,7 +105,9 @@ fn detect_new_conda_env() {
let env = EnvironmentApi::new();

let conda = Conda::from(&env);
let result = conda.find().unwrap();
let reporter = create_reporter();
conda.find(&reporter);
let result = reporter.get_result();

assert_eq!(result.managers.len(), 1);

Expand Down Expand Up @@ -191,14 +196,17 @@ fn detect_new_conda_env_without_python() {
use pet_core::{
os_environment::EnvironmentApi, python_environment::PythonEnvironmentCategory, Locator,
};
use pet_reporter::test::create_reporter;
use std::path::PathBuf;

let env_name = "env_without_python";
create_conda_env(&CondaCreateEnvNameOrPath::Name(env_name.into()), None);
let env = EnvironmentApi::new();

let conda = Conda::from(&env);
let result = conda.find().unwrap();
let reporter = create_reporter();
conda.find(&reporter);
let result = reporter.get_result();

assert_eq!(result.managers.len(), 1);

Expand Down Expand Up @@ -238,6 +246,7 @@ fn detect_new_conda_env_created_with_p_flag_without_python() {
use pet_core::{
os_environment::EnvironmentApi, python_environment::PythonEnvironmentCategory, Locator,
};
use pet_reporter::test::create_reporter;
use std::path::PathBuf;

let env_name = "env_without_python3";
Expand All @@ -246,7 +255,9 @@ fn detect_new_conda_env_created_with_p_flag_without_python() {
let env = EnvironmentApi::new();

let conda = Conda::from(&env);
let result = conda.find().unwrap();
let reporter = create_reporter();
conda.find(&reporter);
let result = reporter.get_result();

assert_eq!(result.managers.len(), 1);

Expand Down Expand Up @@ -286,6 +297,7 @@ fn detect_new_conda_env_created_with_p_flag_with_python() {
use pet_core::{
os_environment::EnvironmentApi, python_environment::PythonEnvironmentCategory, Locator,
};
use pet_reporter::test::create_reporter;
use std::path::PathBuf;

let env_name = "env_with_python3";
Expand All @@ -298,7 +310,9 @@ fn detect_new_conda_env_created_with_p_flag_with_python() {
let env = EnvironmentApi::new();

let conda = Conda::from(&env);
let result = conda.find().unwrap();
let reporter = create_reporter();
conda.find(&reporter);
let result = reporter.get_result();

assert_eq!(result.managers.len(), 1);

Expand Down
3 changes: 2 additions & 1 deletion crates/pet-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use manager::EnvManager;
use pet_utils::env::PythonEnv;
use python_environment::PythonEnvironment;
use reporter::Reporter;

pub mod arch;
pub mod manager;
Expand Down Expand Up @@ -39,5 +40,5 @@ pub trait Locator: Send + Sync {
/**
* Finds all environments specific to this locator.
*/
fn find(&self) -> Option<LocatorResult>;
fn find(&self, reporter: &dyn Reporter);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously we'd report as soon as each locator finds environments.
Thats very fast.
However I can see conda getting slower and slower.
E.g. everything completes in < 50ms for me, except for conda.
Now that I have installed 30 conda envs, and 8 conda installs, it takes 100-200ms
So, first conda gets reported only after all conda envs have been disccovered.

The change is to now report immediately as they are discovered.
Perf is still the same, perhaps slightly faster,

}
Loading