feat: Add channel priority to solve task and expose to python solve #598

BenjaminLowry · 2024-04-08T17:36:25Z

There are use cases where disabling strict channel priority is important and so should have functionality in rattler such that it can be disabled. So I added it as a field of the SolveTask and exposed it as a new parameter (that existed before but was deleted) in py-rattler.

BenjaminLowry · 2024-04-08T17:58:52Z

crates/rattler_solve/src/libsolv_c/mod.rs

@@ -96,6 +96,12 @@ impl super::SolverImpl for Solver {
                "timeout".to_string()
            ]));
        }
+        // Could ignore the strict_channel_priority flag here or do something like below maybe?


This part I'm not so sure what we'd want the behavior to be, it seems to me that this libsolv code doesn't enforce strict channel priority? If so, we can either ignore the option all together to throw if strict priority is true since that's "not supported"?

@wolfv Libsolv supports this too right? Can you point to where this is handled in mamba?

Yep. Probably we need to set the priority on the repository, though? I'll take a look.

BenjaminLowry · 2024-04-08T18:09:31Z

@ruben-arts here is my PR that we discussed earlier today, let me know what you think

baszalmstra

I believe conda has more options, @wolfv is that the case? In that case an enum would be nicer.

baszalmstra · 2024-04-08T18:21:47Z

crates/rattler_solve/src/libsolv_c/mod.rs

@@ -96,6 +96,12 @@ impl super::SolverImpl for Solver {
                "timeout".to_string()
            ]));
        }
+        // Could ignore the strict_channel_priority flag here or do something like below maybe?


@wolfv Libsolv supports this too right? Can you point to where this is handled in mamba?

BenjaminLowry · 2024-04-09T14:19:33Z

@baszalmstra It does look like conda has three options: strict, flexible and disabled https://conda.io/projects/conda/en/latest/user-guide/tasks/manage-channels.html#strict-channel-priority and they default to flexible. Rattler via resolvo is defaulting to strict. I can add an enum but maybe with only two values, strict and disabled for now? And flexible could be added later if needed?

And I see there is a flag for strict priority for libsolv that micromamba uses but I'm a bit ignorant to how this would be translated to this code.

baszalmstra · 2024-04-09T16:10:08Z

Sounds good! Maybe wolf can help with the libsolv side.

BenjaminLowry · 2024-04-09T22:26:26Z

@baszalmstra @wolfv I thought that I figured out how to pass the flag to libsolv in this commit 54796e7 but it's not failing the test that I added in this commit like resolvo does. It shouldn't be able to solve for that version of pytorch-cpu since it's only present in the pytorch channel. Am I doing something wrong on the libsolv side? Or maybe libsolv's strict priority isn't as strict somehow? I'm fairly stumped

BenjaminLowry · 2024-04-11T14:52:28Z

@baszalmstra @wolfv Sorry for the repeated pings, this is just a fairly important change for our workflows.

If we can't resolve why libsolv isn't behaving expectedly with strict channel priority, I can just throw if someone specifies strict when using libsolv? And strict handling could be added later

wolfv · 2024-04-15T15:08:33Z

crates/rattler_solve/src/lib.rs

+    /// for that package.
+    Strict,
+
+    /// Conda also has "Flexible" as an option, where packages present in multiple channels


maybe we shoudl use // so that these comments do not show up in teh documentation for a feature we don't implement.

Yeah sounds good, I only did that cuz with // it formats the comments right against the disabled comment, but likely more confusing for it to be in the documentation

wolfv

Just rethinking the naming. Let me take a quick look at libsolv!

wolfv · 2024-04-15T15:09:02Z

crates/rattler_solve/src/lib.rs

+    /// Flexible,
+
+    /// Packages can be retrieved from any channel as package version takes precedence.
+    Disabled,


Shoudl we call this AllEqual or NoPriority/NoPriorities? Just thinking of something that makes it really clear.

I was thinking it was nice to be consistent with conda, but also happy to change to NoPriority if you prefer

Yeah, totally. I am also OK with keeping the naming. Just sometimes I think we have the opportunity to come up with new & improved terminology :)

wolfv · 2024-04-15T15:09:49Z

crates/rattler_solve/src/libsolv_c/mod.rs

@@ -96,6 +96,12 @@ impl super::SolverImpl for Solver {
                "timeout".to_string()
            ]));
        }
+        // Could ignore the strict_channel_priority flag here or do something like below maybe?


Yep. Probably we need to set the priority on the repository, though? I'll take a look.

wolfv · 2024-04-15T15:38:44Z

@BenjaminLowry I made a PR against your branch here: BenjaminLowry#1. It shows how we could set the priority for libsolv.

Libsolv has a priority and a subpriority integer value. The higher priority should win. We would also need a map or count down the channels and I haven't yet investigated how to do that with the structures at hand.

BenjaminLowry · 2024-04-19T16:23:25Z

@wolfv Thanks for the PR, I added logic to assign repo priority values where it's determined by the order of the repodatas passed in where the channel with the first repodata is given the highest priority and it descends from there. This is pre-computed so we don't have to do some arbitrary highest priority value and so if you have 5 channels the first will be given 5 and the last will be given 1 and 0 is still available for lowest priority use.

This is now behaving as expected for libsolv with strict and disabled priority, does this implementation seem reasonable? Let me know if I'm not following specific rust-isms as well

…-priority

baszalmstra

LGTM!

BenjaminLowry · 2024-04-22T15:45:08Z

@baszalmstra I'm assuming you also want @wolfv to give a +1? Or is this good to go

baszalmstra · 2024-04-22T15:52:59Z

Yeah since @wolfv has a bunch of open comments I was waiting for his approval as well. 😄

Wolf if you are reading this, can you merge if you are happy with this?

wolfv · 2024-04-25T08:07:10Z

crates/rattler_solve/src/libsolv_c/mod.rs

-            let repo = ManuallyDrop::new(Repo::new(&pool, channel_name));
+            // let repo = ManuallyDrop::new(Repo::new(&pool, channel_name));
+            let priority: i32 = if task.channel_priority == ChannelPriority::Strict {
+                // TODO we need to fill in the correct values here, derived from some channel -> priority map


These comments are outdated, right?

wolfv · 2024-04-25T08:07:19Z

crates/rattler_solve/src/libsolv_c/mod.rs

            let channel_name = &repodata.records[0].channel;

            // We dont want to drop the Repo, its stored in the pool anyway.
-            let repo = ManuallyDrop::new(Repo::new(&pool, channel_name));
+            // let repo = ManuallyDrop::new(Repo::new(&pool, channel_name));


Keep or remove?

BenjaminLowry · 2024-04-25T12:27:26Z

@wolfv I had removed those comments but not committed the change 🤦 now they're gone!

BenjaminLowry · 2024-04-25T15:27:15Z

@baszalmstra Thanks for already working on a release! I saw that py-rattler was bumped to 0.4.0 yesterday, will we be able to ship this change in a new release of py-rattler soon?

baszalmstra · 2024-04-25T15:37:52Z

Sure! I can make another release in the next few days.

BenjaminLowry · 2024-04-25T15:46:27Z

Amazing thanks!

baszalmstra · 2024-04-26T09:11:28Z

py-rattler 0.5.0 is on pypi which includes your changes.

add flag for strict channel priority in solve task

7d269d0

BenjaminLowry changed the title ~~Add option to disable strict channel priority in solve task~~ feat: Add option to disable strict channel priority in solve task Apr 8, 2024

BenjaminLowry added 2 commits April 8, 2024 13:46

format

ef99f42

rust fmt

a58fa8b

BenjaminLowry commented Apr 8, 2024

View reviewed changes

fix

ca9ffb2

baszalmstra reviewed Apr 8, 2024

View reviewed changes

BenjaminLowry added 5 commits April 9, 2024 16:51

use enums

2a2d93c

remove unused

013db67

add back

3a2900b

libsolv strict priority flag

54796e7

fmt

37172ca

wolfv reviewed Apr 15, 2024

View reviewed changes

BenjaminLowry added 2 commits April 19, 2024 11:43

fix libsolv priority

2c41e2a

fix priority for pinned/locked

176b870

Merge remote-tracking branch 'origin/main' into blowry/strict-channel…

5af8dbf

…-priority

baszalmstra approved these changes Apr 19, 2024

View reviewed changes

BenjaminLowry changed the title ~~feat: Add option to disable strict channel priority in solve task~~ feat: Add channel priority to solve task and expose to python solve Apr 19, 2024

wolfv mentioned this pull request Apr 22, 2024

Allow flexible channel priority prefix-dev/pixi#1254

Closed

BenjaminLowry requested a review from wolfv April 24, 2024 14:23

wolfv reviewed Apr 25, 2024

View reviewed changes

remove comments

57073d9

BenjaminLowry requested a review from wolfv April 25, 2024 12:26

wolfv approved these changes Apr 25, 2024

View reviewed changes

wolfv merged commit 926a876 into conda:main Apr 25, 2024
14 checks passed

baszalmstra mentioned this pull request Apr 25, 2024

chore: release #625

Merged

roaldarbol mentioned this pull request Jul 18, 2024

Command to create new Pixi environments from requirements.txt or environment.yml files prefix-dev/pixi#1410

Open

jpcorreia99 mentioned this pull request Dec 11, 2024

Feature: Support flexible priority in solve task #976

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add channel priority to solve task and expose to python solve #598

feat: Add channel priority to solve task and expose to python solve #598

BenjaminLowry commented Apr 8, 2024

BenjaminLowry Apr 8, 2024

baszalmstra Apr 8, 2024

wolfv Apr 15, 2024

BenjaminLowry commented Apr 8, 2024

baszalmstra left a comment

baszalmstra Apr 8, 2024

BenjaminLowry commented Apr 9, 2024

baszalmstra commented Apr 9, 2024

BenjaminLowry commented Apr 9, 2024 •

edited

Loading

BenjaminLowry commented Apr 11, 2024

wolfv Apr 15, 2024

BenjaminLowry Apr 15, 2024

wolfv left a comment

wolfv Apr 15, 2024

BenjaminLowry Apr 15, 2024

wolfv Apr 15, 2024

wolfv Apr 15, 2024

wolfv commented Apr 15, 2024 •

edited

Loading

BenjaminLowry commented Apr 19, 2024

baszalmstra left a comment

BenjaminLowry commented Apr 22, 2024

baszalmstra commented Apr 22, 2024

wolfv Apr 25, 2024

wolfv Apr 25, 2024

BenjaminLowry commented Apr 25, 2024

BenjaminLowry commented Apr 25, 2024

baszalmstra commented Apr 25, 2024

BenjaminLowry commented Apr 25, 2024

baszalmstra commented Apr 26, 2024 •

edited

Loading

feat: Add channel priority to solve task and expose to python solve #598

feat: Add channel priority to solve task and expose to python solve #598

Conversation

BenjaminLowry commented Apr 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenjaminLowry commented Apr 8, 2024

baszalmstra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenjaminLowry commented Apr 9, 2024

baszalmstra commented Apr 9, 2024

BenjaminLowry commented Apr 9, 2024 • edited Loading

BenjaminLowry commented Apr 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wolfv left a 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

wolfv commented Apr 15, 2024 • edited Loading

BenjaminLowry commented Apr 19, 2024

baszalmstra left a comment

Choose a reason for hiding this comment

BenjaminLowry commented Apr 22, 2024

baszalmstra commented Apr 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenjaminLowry commented Apr 25, 2024

BenjaminLowry commented Apr 25, 2024

baszalmstra commented Apr 25, 2024

BenjaminLowry commented Apr 25, 2024

baszalmstra commented Apr 26, 2024 • edited Loading

BenjaminLowry commented Apr 9, 2024 •

edited

Loading

wolfv commented Apr 15, 2024 •

edited

Loading

baszalmstra commented Apr 26, 2024 •

edited

Loading