feat: start adding interface to override #834

SobhanMP · 2024-08-22T02:07:33Z

Following 721a6c1, this commit will start adding the interface for python.

SobhanMP · 2024-08-22T02:10:34Z

@baszalmstra Should VirtualPackageOverride be accessible from python?

iamthebot · 2024-08-26T19:45:22Z

@baszalmstra Should VirtualPackageOverride be accessible from python?

@SobhanMP we'd use this-- can you include it?

SobhanMP · 2024-08-26T23:17:52Z

@iamthebot (we'd have to ask @baszalmstra), but as things are, the overrides are enabled by default, so they work. Do you need to rename the overrides?

iamthebot · 2024-08-27T04:09:54Z

@SobhanMP

Sorry, I think I misunderstood. We'd like to be able to specify the overrides for eg; CUDA directly without needing to use an env var for the override. If I'm reading this correctly-- as written, the overrides would work via env var directly but without exposing VirtualPackageOverride we can't control that override directly from Python side.

baszalmstra · 2024-08-27T04:12:28Z

crates/rattler_virtual_packages/src/lib.rs

+        static DETECTED_VIRTUAL_PACKAGES: OnceCell<Vec<VirtualPackage>> = OnceCell::new();
+        DETECTED_VIRTUAL_PACKAGES


This caches the values but based on the input which means if you call it a second time with different input you get the same result as rhe first time. I think it would be better to not cache the values here and return a Vec.

Good point.

baszalmstra · 2024-08-27T04:13:45Z

crates/rattler_virtual_packages/src/lib.rs

@@ -136,6 +170,24 @@ impl VirtualPackage {
            .get_or_try_init(try_detect_virtual_packages)
            .map(Vec::as_slice)
    }
+
+    /// disable overrides
+    pub fn current_no_overrides() -> Result<&'static [Self], DetectVirtualPackageError> {


Isnt this just the same as the current method?

I changed the default to match conda (i.e. use the default overrides)

baszalmstra · 2024-08-27T04:14:48Z

crates/rattler_virtual_packages/src/lib.rs

+/// Configure the overrides used in in this crate.
+
+pub struct VirtualPackageOverride<'a> {
+    osx: Option<&'a str>,


Perhaps use an Option<String> to remove the need for lifetimes.

I wonder if maybe we should change this to a trie state:

dont use any override

use the default env var if it exists,

use the environment variable with a specific name.

that way we can get rid of all the slightly different functions.

WDYT?

I agree, but how about a 4th option that forces a specific version? (I think @iamthebot would like that), I'll give this a try, and if it doesn't work, we can revert to the last commit.

baszalmstra · 2024-08-27T04:15:17Z

crates/rattler_virtual_packages/src/lib.rs

+    }
+
+    /// Use the default overrides of Conda.
+    fn default() -> Self {


Please implement Default

Default as in the default trait? Will do, did not know it was a thing.

SobhanMP · 2024-08-28T03:17:43Z

@baszalmstra where would be the right place to put the tests? and is this interface/impl ok?

❯ CONDA_OVERRIDE_CUDA=1984 pixi run rattler create jupyterlab
Pixi task (rattler in default): cargo run --bin rattler --release -- create jupyterlab
    Finished release [optimized] target(s) in 0.13s
     Running `.pixi/target/release/rattler create jupyterlab`
Target prefix: /home/psi/rattler/.prefix
Installing for platform: Linux64
Loaded 20237 records in 520.548978ms
Virtual packages:
  - __unix=0=0
  - __linux=6.6.45=0
  - __glibc=2.40=0
  - __cuda=1984=0
  - __archspec=1=x86_64_v4

 Already up to date

baszalmstra · 2024-08-28T07:18:29Z

I dont think we need an end-to-end test. I would place some unit tests in the lib.rs that test a few different overrides. Watch out with environment variables though because rust runs all tests in parallel.

SobhanMP · 2024-08-28T14:36:24Z

@baszalmstra I think this is done (minus minor ci failure), unless overrides should also override checks like platform.is_linux (i.e., should we move the conditionals inside the current function?)

baszalmstra

This is starting to shape up nicely!

crates/rattler_virtual_packages/src/lib.rs

py-rattler/rattler/virtual_package/virtual_package.py

crates/rattler_virtual_packages/src/lib.rs

SobhanMP · 2024-08-29T14:03:10Z

@baszalmstra, I did the renaming and the deprecation. I think that's everything. Apologies if I missed something.

right now:

current is the old current
detect is detect with defaults
VirtualPackageOverrides.__init__ replaces VirtualPackageOverrides.default
Some documentation on the python side
The functions are detect_with_overrides and detect_with_fallback.

baszalmstra · 2024-08-29T17:16:14Z

crates/rattler_virtual_packages/src/lib.rs

+    }
+
+    /// Detect the virtual packages of the current system with the default overrides.
+    pub fn detect() -> Result<Vec<Self>, DetectVirtualPackageError> {


Lets just remove this function in favor of detect_with_overrides. I dont think we need an overload without an argument.

I think
let virtual_packages = rattler_virtual_packages::VirtualPackage::detect_with_overrides(&rattler_virtual_packages::VirtualPackageOverrides::default())?;
is not very concise 😅

VirtualPackage::detect(&Default::default()) seems fine to me.

hmm, yeah, that works. Somehow, Default is more robust than what I imagined.

crates/rattler_virtual_packages/src/lib.rs

py-rattler/tests/unit/test_override.py

py-rattler/rattler/virtual_package/virtual_package.py

Co-authored-by: Bas Zalmstra <zalmstra.bas@gmail.com>

baszalmstra · 2024-08-29T18:14:37Z

py-rattler/rattler/virtual_package/virtual_package.py

+        return VirtualPackage.detect_with_overrides(VirtualPackageOverrides.none())
+
+    @staticmethod
+    def detect_with_overrides(overrides: VirtualPackageOverrides | None = None) -> List[VirtualPackage]:


Maybe we can rename to detect? With or without override is already defined by the argument.

I think this is outdated, right?

No I think we should just have one function called detect which you can pass an override or not. I think its superfluous to also add with_override to the name.

baszalmstra · 2024-08-29T19:50:49Z

crates/rattler_virtual_packages/src/lib.rs

-            .map(Vec::as_slice)
+    #[deprecated(
+        since = "1.0.4",
+        note = "Use `Self::detect(&Default::default)` instead"


mm I guess this should be none right?

I'd argue no, because the expectation is to be conda compatible 🤷

I disagree, rattler is also used in applications where this outside environment override behavior is unwanted. For an application this behavior makes sense but for a library I think reading from environment variables should be opt in.

Furthermore, this suggestion changes the default current behavior of the function. I think the depreciation should tell people how to migrate to something which exhibits the same behavior.

baszalmstra · 2024-08-30T17:45:34Z

Thanks! Sorry it took so long!

SobhanMP · 2024-08-30T17:45:48Z

No worries, thanks for all the helpful comments!

## 🤖 New release * `rattler_conda_types`: 0.27.2 -> 0.27.3 * `rattler_package_streaming`: 0.22.3 -> 0.22.4 * `rattler_lock`: 0.22.20 -> 0.22.21 * `rattler_solve`: 1.0.3 -> 1.0.4 * `rattler_virtual_packages`: 1.0.4 -> 1.1.0 * `rattler_index`: 0.19.24 -> 0.19.25 * `rattler`: 0.27.6 -> 0.27.7 * `rattler_cache`: 0.1.8 -> 0.1.9 * `rattler_shell`: 0.21.6 -> 0.21.7 * `rattler_repodata_gateway`: 0.21.8 -> 0.21.9 <details><summary>Changelog</summary> ## `rattler_conda_types` <blockquote> ## [0.27.3](rattler_conda_types-v0.27.2...rattler_conda_types-v0.27.3) - 2024-09-02 ### Added - add edge case tests for `StringMatcher` ([#839](#839)) </blockquote> ## `rattler_package_streaming` <blockquote> ## [0.22.4](rattler_package_streaming-v0.22.3...rattler_package_streaming-v0.22.4) - 2024-09-02 ### Added - Add support for `CONDA_OVERRIDE_CUDA` ([#818](#818)) ### Fixed - zip large files compression ([#838](#838)) </blockquote> ## `rattler_lock` <blockquote> ## [0.22.21](rattler_lock-v0.22.20...rattler_lock-v0.22.21) - 2024-09-02 ### Added - Add support for `CONDA_OVERRIDE_CUDA` ([#818](#818)) </blockquote> ## `rattler_solve` <blockquote> ## [1.0.4](rattler_solve-v1.0.3...rattler_solve-v1.0.4) - 2024-09-02 ### Fixed - Redact spec channel before comparing it with repodata channel ([#831](#831)) ### Other - Remove note that only libsolv is supported ([#832](#832)) </blockquote> ## `rattler_virtual_packages` <blockquote> ## [1.1.0](rattler_virtual_packages-v1.0.4...rattler_virtual_packages-v1.1.0) - 2024-09-02 ### Added - start adding interface to override ([#834](#834)) - Add support for `CONDA_OVERRIDE_CUDA` ([#818](#818)) ### Other - make virtual package overrides none by default consistently ([#842](#842)) </blockquote> ## `rattler_index` <blockquote> ## [0.19.25](rattler_index-v0.19.24...rattler_index-v0.19.25) - 2024-09-02 ### Other - release ([#824](#824)) </blockquote> ## `rattler` <blockquote> ## [0.27.7](rattler-v0.27.6...rattler-v0.27.7) - 2024-09-02 ### Other - updated the following local packages: rattler_conda_types, rattler_package_streaming </blockquote> ## `rattler_cache` <blockquote> ## [0.1.9](rattler_cache-v0.1.8...rattler_cache-v0.1.9) - 2024-09-02 ### Other - updated the following local packages: rattler_conda_types, rattler_package_streaming </blockquote> ## `rattler_shell` <blockquote> ## [0.21.7](rattler_shell-v0.21.6...rattler_shell-v0.21.7) - 2024-09-02 ### Other - updated the following local packages: rattler_conda_types </blockquote> ## `rattler_repodata_gateway` <blockquote> ## [0.21.9](rattler_repodata_gateway-v0.21.8...rattler_repodata_gateway-v0.21.9) - 2024-09-02 ### Other - updated the following local packages: rattler_conda_types </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/).

## 🤖 New release * `rattler_conda_types`: 0.27.2 -> 0.27.3 * `rattler_package_streaming`: 0.22.3 -> 0.22.4 * `rattler_lock`: 0.22.20 -> 0.22.21 * `rattler_solve`: 1.0.3 -> 1.0.4 * `rattler_virtual_packages`: 1.0.4 -> 1.1.0 * `rattler_index`: 0.19.24 -> 0.19.25 * `rattler`: 0.27.6 -> 0.27.7 * `rattler_cache`: 0.1.8 -> 0.1.9 * `rattler_shell`: 0.21.6 -> 0.21.7 * `rattler_repodata_gateway`: 0.21.8 -> 0.21.9 <details><summary>Changelog</summary> ## `rattler_conda_types` <blockquote> ## [0.27.3](conda/rattler@rattler_conda_types-v0.27.2...rattler_conda_types-v0.27.3) - 2024-09-02 ### Added - add edge case tests for `StringMatcher` ([conda#839](conda#839)) </blockquote> ## `rattler_package_streaming` <blockquote> ## [0.22.4](conda/rattler@rattler_package_streaming-v0.22.3...rattler_package_streaming-v0.22.4) - 2024-09-02 ### Added - Add support for `CONDA_OVERRIDE_CUDA` ([conda#818](conda#818)) ### Fixed - zip large files compression ([conda#838](conda#838)) </blockquote> ## `rattler_lock` <blockquote> ## [0.22.21](conda/rattler@rattler_lock-v0.22.20...rattler_lock-v0.22.21) - 2024-09-02 ### Added - Add support for `CONDA_OVERRIDE_CUDA` ([conda#818](conda#818)) </blockquote> ## `rattler_solve` <blockquote> ## [1.0.4](conda/rattler@rattler_solve-v1.0.3...rattler_solve-v1.0.4) - 2024-09-02 ### Fixed - Redact spec channel before comparing it with repodata channel ([conda#831](conda#831)) ### Other - Remove note that only libsolv is supported ([conda#832](conda#832)) </blockquote> ## `rattler_virtual_packages` <blockquote> ## [1.1.0](conda/rattler@rattler_virtual_packages-v1.0.4...rattler_virtual_packages-v1.1.0) - 2024-09-02 ### Added - start adding interface to override ([conda#834](conda#834)) - Add support for `CONDA_OVERRIDE_CUDA` ([conda#818](conda#818)) ### Other - make virtual package overrides none by default consistently ([conda#842](conda#842)) </blockquote> ## `rattler_index` <blockquote> ## [0.19.25](conda/rattler@rattler_index-v0.19.24...rattler_index-v0.19.25) - 2024-09-02 ### Other - release ([conda#824](conda#824)) </blockquote> ## `rattler` <blockquote> ## [0.27.7](conda/rattler@rattler-v0.27.6...rattler-v0.27.7) - 2024-09-02 ### Other - updated the following local packages: rattler_conda_types, rattler_package_streaming </blockquote> ## `rattler_cache` <blockquote> ## [0.1.9](conda/rattler@rattler_cache-v0.1.8...rattler_cache-v0.1.9) - 2024-09-02 ### Other - updated the following local packages: rattler_conda_types, rattler_package_streaming </blockquote> ## `rattler_shell` <blockquote> ## [0.21.7](conda/rattler@rattler_shell-v0.21.6...rattler_shell-v0.21.7) - 2024-09-02 ### Other - updated the following local packages: rattler_conda_types </blockquote> ## `rattler_repodata_gateway` <blockquote> ## [0.21.9](conda/rattler@rattler_repodata_gateway-v0.21.8...rattler_repodata_gateway-v0.21.9) - 2024-09-02 ### Other - updated the following local packages: rattler_conda_types </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/).

start adding interface to override

726c633

SobhanMP changed the title ~~start adding interface to override~~ feat: start adding interface to override Aug 22, 2024

SobhanMP added 3 commits August 21, 2024 22:15

complete the interface

02d1fc8

handle empty strings again

84b362b

fix test

aa8e7e1

baszalmstra reviewed Aug 27, 2024

View reviewed changes

SobhanMP added 8 commits August 27, 2024 11:41

iter

6732316

remove unused import

16beb97

Merge branch 'main' into env

c32b28c

use as_deref

de3f331

add quad state overrides

2a9ffdd

fmt

8fad46f

remove useless methods

8bbd546

make python binding

28e07ac

SobhanMP added 4 commits August 28, 2024 09:35

add a few more tests, do not use default env var name, just in case

1e030e9

add a single python unit test

06dfb06

add a python test for good measure

2dc4577

add annotation

3810861

SobhanMP added 3 commits August 28, 2024 12:03

fix doc

def7eb6

annotate test

f4a9803

fmt

184f1a4

SobhanMP requested a review from baszalmstra August 28, 2024 16:31

baszalmstra reviewed Aug 29, 2024

View reviewed changes

SobhanMP added 2 commits August 29, 2024 09:58

rename some stuff

a7cabed

Merge branch 'env' of github.com:SobhanMP/rattler into env

d83dfb7

SobhanMP added 5 commits August 29, 2024 10:08

fmt, stop using current

8f17402

bump

1618ae0

mypy

a6a8403

ruf

752bdd2

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

976c30f

SobhanMP requested a review from baszalmstra August 29, 2024 15:24

baszalmstra reviewed Aug 29, 2024

View reviewed changes

SobhanMP and others added 5 commits August 29, 2024 13:25

Update crates/rattler_virtual_packages/src/lib.rs

fb21bf4

Co-authored-by: Bas Zalmstra <zalmstra.bas@gmail.com>

apply @Baszalmastra's suggetions

b79dde0

use Default::default()

d6c8d59

fmt

157c813

fix test

1c03db5

SobhanMP requested a review from baszalmstra August 29, 2024 19:21

baszalmstra reviewed Aug 29, 2024

View reviewed changes

SobhanMP added 2 commits August 29, 2024 16:07

change deprecation

9057932

oops

5aa6af8

SobhanMP requested a review from baszalmstra August 29, 2024 20:22

baszalmstra approved these changes Aug 30, 2024

View reviewed changes

Merge branch 'main' into env

24b24f8

baszalmstra enabled auto-merge (squash) August 30, 2024 17:46

baszalmstra merged commit 8a9fed3 into conda:main Aug 30, 2024
15 checks passed

baszalmstra mentioned this pull request Aug 30, 2024

chore: release #836

Merged

SobhanMP deleted the env branch August 30, 2024 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: start adding interface to override #834

feat: start adding interface to override #834

SobhanMP commented Aug 22, 2024 •

edited

Loading

SobhanMP commented Aug 22, 2024

iamthebot commented Aug 26, 2024

SobhanMP commented Aug 26, 2024

iamthebot commented Aug 27, 2024

baszalmstra Aug 27, 2024

SobhanMP Aug 27, 2024

baszalmstra Aug 27, 2024

SobhanMP Aug 27, 2024

baszalmstra Aug 27, 2024

baszalmstra Aug 27, 2024

SobhanMP Aug 27, 2024 •

edited

Loading

baszalmstra Aug 27, 2024

SobhanMP Aug 27, 2024

SobhanMP commented Aug 28, 2024 •

edited

Loading

baszalmstra commented Aug 28, 2024

SobhanMP commented Aug 28, 2024 •

edited

Loading

baszalmstra left a comment

SobhanMP commented Aug 29, 2024

baszalmstra Aug 29, 2024

SobhanMP Aug 29, 2024

baszalmstra Aug 29, 2024

SobhanMP Aug 29, 2024

baszalmstra Aug 29, 2024

SobhanMP Aug 29, 2024 •

edited

Loading

baszalmstra Aug 29, 2024

baszalmstra Aug 29, 2024

SobhanMP Aug 29, 2024

baszalmstra Aug 29, 2024

baszalmstra Aug 29, 2024

baszalmstra commented Aug 30, 2024

SobhanMP commented Aug 30, 2024

		static DETECTED_VIRTUAL_PACKAGES: OnceCell<Vec<VirtualPackage>> = OnceCell::new();
		DETECTED_VIRTUAL_PACKAGES

feat: start adding interface to override #834

feat: start adding interface to override #834

Conversation

SobhanMP commented Aug 22, 2024 • edited Loading

SobhanMP commented Aug 22, 2024

iamthebot commented Aug 26, 2024

SobhanMP commented Aug 26, 2024

iamthebot commented Aug 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SobhanMP Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SobhanMP commented Aug 28, 2024 • edited Loading

baszalmstra commented Aug 28, 2024

SobhanMP commented Aug 28, 2024 • edited Loading

baszalmstra left a comment

Choose a reason for hiding this comment

SobhanMP commented Aug 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SobhanMP Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baszalmstra commented Aug 30, 2024

SobhanMP commented Aug 30, 2024

SobhanMP commented Aug 22, 2024 •

edited

Loading

SobhanMP Aug 27, 2024 •

edited

Loading

SobhanMP commented Aug 28, 2024 •

edited

Loading

SobhanMP commented Aug 28, 2024 •

edited

Loading

SobhanMP Aug 29, 2024 •

edited

Loading