Fix OCIMirror use #3296
Fix OCIMirror use #3296
Conversation
Hind-M
added
the
release::enhancements
Add back curl config
| @@ -324,26 +325,12 @@ namespace mamba | |||
|
|
|||
| std::string PackageFetcher::channel() const | |||
| { | |||
| if (m_package_info.package_type == specs::PackageType::Conda) | |||
There was a problem hiding this comment.
Not sure why this was added but the package type is set to its default value Unknown while it should be set to Conda when downloading using OCI mirrors.
That said, changing this (in this PR) still makes the tests pass in the CI, so I don't know what's the use case of the else...
Same for url_path()
There was a problem hiding this comment.
I don't remember either Oo
| logger->info(fmt::format("< {}", log)); | ||
| break; | ||
| default: | ||
| // WARNING Using `hide_secrets` here will give a seg fault on linux, |
There was a problem hiding this comment.
I actually found out that the seg fault was related to the default case, so getting hide_secrets back in the other cases and adding a warning in the default one.
| } | ||
|
|
||
| // This is not used but could be if we use creds | ||
| // cf. OCIMirror constructor comment in header | ||
| bool OCIMirror::need_authentication() const |
There was a problem hiding this comment.
Removing need_authentication() from where it was used to avoid confusion as it's not clear how the credentials are supposed to be used. To pull artifacts we request a token, and we don't support uploading like powerloader, so AFAIK there is no need for credentials at the moment.
I also added this comment for more details.
I left the function implementation though even if it's not used (as a hint for maybe later if needed, like username and password) but we could also decide this is useless and just remove it/them.
|
🎉 |
| @@ -324,26 +325,12 @@ namespace mamba | |||
|
|
|||
| std::string PackageFetcher::channel() const | |||
| { | |||
| if (m_package_info.package_type == specs::PackageType::Conda) | |||
There was a problem hiding this comment.
I don't remember either Oo
| // containing the `checksum` and its `type` | ||
| // (to handle other checksums types like md5...) | ||
| // cf. `Checksum` struct in powerloader | ||
| std::string sha256; |
There was a problem hiding this comment.
I think we want something even more generic to handle additional parameters specific to a kind of storage. This should be done in a dedicated PR, though.
|
Bravo! Congrats on getting this in @Hind-M 🎉 🎉 🎉 🎉 🎉 🎉 |
No description provided.