feat: use new mapping #888

nichmor · 2024-02-29T16:16:29Z

No description provided.

src/pypi_name_mapping.rs

src/lock_file/update.rs

src/pypi_name_mapping.rs

baszalmstra · 2024-03-08T19:15:21Z

Before we merge this we want to make sure we open source the repository that generates the mapping.

ruben-arts

Could you also use the mapping in lock_file::pypi::resolve_dependencies so we don't add the wrong packages to the resolve.

src/pypi_name_mapping.rs

wolfv · 2024-03-20T14:33:12Z

I just rebased this PR on main.

A few things I am wondering:

should we even retry fetching the mapping? I think in some corporate environments this will be blocked etc, so retrying will be futile :)
we just discussed via voice that we do not store a mapping if the conda package has no corresponding pypi package. In that case it seems like we'd be retrying 3 times now for a file that doesn't exist. That sounds slow.

I think we might be better off not retrying and it might also be good to have some graceful fallback - maybe with the current github file, or something hard-coded?

nichmor · 2024-03-20T16:33:43Z

I just rebased this PR on main.

A few things I am wondering:

should we even retry fetching the mapping? I think in some corporate environments this will be blocked etc, so retrying will be futile :)

we just discussed via voice that we do not store a mapping if the conda package has no corresponding pypi package. In that case it seems like we'd be retrying 3 times now for a file that doesn't exist. That sounds slow.

I think we might be better off not retrying and it might also be good to have some graceful fallback - maybe with the current github file, or something hard-coded?

What do you mean by hard-coded/current github file?

baszalmstra · 2024-03-20T16:42:06Z

Retry with the middleware only happens for 500 errors and same rare 42x cases. A missing entry will not be retried.

I assume corporate firewalls will also return a normal 40x status which will also not be retried.

The main usecase for retry is to retry server issues.

baszalmstra · 2024-03-20T16:43:04Z

See: https://docs.rs/reqwest-retry/latest/src/reqwest_retry/retryable_strategy.rs.html#113

wolfv · 2024-03-20T16:50:38Z

Ah great, good to know!

ruben-arts · 2024-03-25T08:11:16Z

Could you also add documentation on the use and working of this mapping?

src/project/manifest/mod.rs

src/project/mod.rs

baszalmstra · 2024-04-02T10:41:32Z

src/project/manifest/metadata.rs

@@ -50,4 +50,7 @@ pub struct ProjectMetadata {

    /// URL of the project documentation
    pub documentation: Option<Url>,
+
+    /// URL of the pypi name mapping
+    pub pypi_name_mapping: Option<Url>,


Im thinking that maybe we should have a mapping per channel. So you can use the prefix one for conda-forge but use a custom mapping for your own private channel?

WDYT?

good idea!
some small discovery questions about how do you see it:

do we want to also allow to override conda-forge with your own mapping?

do we want to expose configuration via pypi-name-mapping = {"conda-forge": ..my_path, "my-own-channel": ..my_another_path } , or to hide the channel in the mapping itself? ( I go with the first approach )

do we want to also allow to override conda-forge with your own mapping?

When we have mirrors or you are behind a firewall, then yes.

do we want to expose configuration via pypi-name-mapping = {"conda-forge": ..my_path, "my-own-channel": ..my_another_path } , or to hide the channel in the mapping itself? ( I go with the first approach )

@ruben-arts @wolfv WDYT?

I like this!

Edit: Scrap the seperate table idea, this is a standalone definition specific to the project. So this is good.

[project] pypi-name-mapping = { conda-forge = "url", "https://prefix.dev/my_channel" = "url" }

I'm having some doubts on the name. What about:

conda-pypi-map

channel-index-map

conda-pypi-package-map

What about starting a [pypi-options] table? I feel there could be a number of future options to handle (keyring, extra index, etc.)

Not sure re. the specific name of the map, but we could maybe take 'pypi' out of it if it sits under a [pypi-options] table

What about starting a [pypi-options] table? I feel there could be a number of future options to handle (keyring, extra index, etc.)

Not sure re. the specific name of the map, but we could maybe take 'pypi' out of it if it sits under a [pypi-options] table

Hey @olivier-lacroix ! Good question!

My rationale for using [project] is that mapping is used for both conda and pypi - so in my view because we map conda name to python name, the upstream for it is the [project] table.
I'm totally open for any other ideas - maybe we can also ask @ruben-arts and @baszalmstra

I agree with @nichmor, not a bad idea but not required in this case.

docs/configuration.md

examples/conda_mapping/test_conda_mapping.py

src/project/manifest/metadata.rs

src/pypi_mapping/custom_pypi_mapping.rs

baszalmstra

This looks pretty good to me except that if I read it correctly you are only using the prefix mapping by default for packages from conda-forge. However, you can use the prefix mapping for all packages!

src/project/manifest/mod.rs

src/pypi_mapping/custom_pypi_mapping.rs

baszalmstra · 2024-04-05T14:19:50Z

I approve of this PR! @ruben-arts If you are happy with it, feel free to merge :)

ruben-arts · 2024-04-05T14:50:47Z

src/pypi_mapping/custom_pypi_mapping.rs

+                        let response = client
+                            .get(url.clone())
+                            .send()
+                            .await
+                            .into_diagnostic()
+                            .context("failed to download pypi mapping from custom location")?;


I tested what happens if you give a broken URL but then this doesn't get shown but then next one does. Could you make sure that it fails on downloading, and that it adds atleast also adds the url to the error so it helps the user find the cultprit? Now it gives this error:

× failed to parse pypi name mapping ├─▶ error decoding response body: invalid type: integer `404`, expected a map at line 1 column 3 ╰─▶ invalid type: integer `404`, expected a map at line 1 column 3

change the logic so know it will throw following error if requested mapping could not be located

and following error if json is bad formatted ( for example, could not be able to serialized in our struct )

ruben-arts · 2024-04-05T15:00:41Z

src/project/manifest/mod.rs

+                        })
+                        .collect::<miette::Result<HashMap<ChannelName, MappingLocation>>>()?;
+
+                Ok(MappingSource::Custom { mapping })


I think it's better to also warn the user if the have a mapping to conda-flibediefloep that it tells them that that channel doesn't exist.

Changed the logic a little so know it will print a warning if defined channel is not present in project.channels

src/pypi_mapping/custom_pypi_mapping.rs

tests/test_examples.sh

ruben-arts

Awesome

baszalmstra reviewed Mar 7, 2024

View reviewed changes

src/pypi_name_mapping.rs Outdated Show resolved Hide resolved

nichmor marked this pull request as ready for review March 7, 2024 21:24

baszalmstra reviewed Mar 7, 2024

View reviewed changes

src/lock_file/update.rs Outdated Show resolved Hide resolved

src/pypi_name_mapping.rs Outdated Show resolved Hide resolved

baszalmstra reviewed Mar 7, 2024

View reviewed changes

src/pypi_name_mapping.rs Outdated Show resolved Hide resolved

ruben-arts requested changes Mar 15, 2024

View reviewed changes

src/pypi_name_mapping.rs Outdated Show resolved Hide resolved

ruben-arts assigned nichmor Mar 15, 2024

implement support for new pypi mapping

5ba6042

wolfv force-pushed the feat/use-new-mapping branch from f051f66 to 5ba6042 Compare March 20, 2024 14:19

baszalmstra and others added 2 commits March 21, 2024 15:30

feat: update progress bars to be cooler

a4d6c0c

Merge branch 'main' into feat/use-new-mapping

e850bfd

This was referenced Mar 25, 2024

feat: Support pyproject.toml #999

Merged

Install dependencies of pypi package from conda preferentially #532

Open

nichmor added 2 commits March 30, 2024 19:48

feat: add custom location for pypi mapping

84fa5a7

misc: don't add package if it's not in mapping

af9c58f

baszalmstra reviewed Apr 2, 2024

View reviewed changes

nichmor added 5 commits April 4, 2024 20:10

feat: add channels for mapping

dad3b0f

misc: rename mapping attribute

087d348

misc: rename manifest mapping name

eb9cafd

misc: add some documentation

3f3534d

Merge branch 'main' into feat/use-new-mapping

6a48243

ruben-arts reviewed Apr 5, 2024

View reviewed changes

nichmor added 2 commits April 5, 2024 13:22

misc: apply review changes

9082078

misc: update toml.schema

4399596

baszalmstra requested changes Apr 5, 2024

View reviewed changes

nichmor added 5 commits April 5, 2024 15:40

misc: update documentation

86195e8

misc: remove is conda-forge check

3b6d842

misc: apply review changes

fd3c61f

misc: update indicatiff

967b34b

misc: fix set style in progress bar

90794db

nichmor requested review from ruben-arts and baszalmstra April 5, 2024 13:09

baszalmstra reviewed Apr 5, 2024

View reviewed changes

src/project/manifest/mod.rs Outdated Show resolved Hide resolved

src/pypi_mapping/custom_pypi_mapping.rs Outdated Show resolved Hide resolved

nichmor added 2 commits April 5, 2024 16:28

misc: upgrade the pixi.lock

08048ff

misc: remove unnecesary conda-forge filter

d6378c6

baszalmstra approved these changes Apr 5, 2024

View reviewed changes

ruben-arts reviewed Apr 5, 2024

View reviewed changes

obust mentioned this pull request Apr 8, 2024

conda_pypi_name_mapping does not honor the mirror_middleware #1048

Closed

2 tasks

misc: apply changes

03f5863

ruben-arts approved these changes Apr 8, 2024

View reviewed changes

ruben-arts merged commit a8b3fee into prefix-dev:main Apr 8, 2024
27 of 28 checks passed

ruben-arts mentioned this pull request Apr 10, 2024

Conda-forge package and Pypi package with the same name #710

Closed

2 tasks

BrewTestBot mentioned this pull request Apr 10, 2024

pixi 0.19.0 Homebrew/homebrew-core#168524

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use new mapping #888

feat: use new mapping #888

nichmor commented Feb 29, 2024 •

edited

Loading

baszalmstra commented Mar 8, 2024

ruben-arts left a comment

wolfv commented Mar 20, 2024

nichmor commented Mar 20, 2024

baszalmstra commented Mar 20, 2024

baszalmstra commented Mar 20, 2024

wolfv commented Mar 20, 2024

ruben-arts commented Mar 25, 2024

baszalmstra Apr 2, 2024

nichmor Apr 2, 2024

baszalmstra Apr 2, 2024

ruben-arts Apr 3, 2024 •

edited

Loading

ruben-arts Apr 3, 2024

olivier-lacroix Apr 5, 2024

nichmor Apr 5, 2024

ruben-arts Apr 5, 2024

baszalmstra left a comment

baszalmstra commented Apr 5, 2024

ruben-arts Apr 5, 2024

nichmor Apr 8, 2024

nichmor Apr 8, 2024

ruben-arts Apr 5, 2024

nichmor Apr 8, 2024

ruben-arts left a comment

feat: use new mapping #888

feat: use new mapping #888

Conversation

nichmor commented Feb 29, 2024 • edited Loading

baszalmstra commented Mar 8, 2024

ruben-arts left a comment

Choose a reason for hiding this comment

wolfv commented Mar 20, 2024

nichmor commented Mar 20, 2024

baszalmstra commented Mar 20, 2024

baszalmstra commented Mar 20, 2024

wolfv commented Mar 20, 2024

ruben-arts commented Mar 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruben-arts Apr 3, 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

baszalmstra left a comment

Choose a reason for hiding this comment

baszalmstra commented Apr 5, 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

ruben-arts left a comment

Choose a reason for hiding this comment

nichmor commented Feb 29, 2024 •

edited

Loading

ruben-arts Apr 3, 2024 •

edited

Loading