-
Notifications
You must be signed in to change notification settings - Fork 101
feat: add host-based upload support to rattler_upload
#1580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c2ecece
7796ddd
d05406c
a490759
9e1a01a
676a6f7
63f33ee
29a1f5c
fc4a790
96ea212
7718b02
cfc2690
9f3b4bb
05e9c12
00d0a45
0bf614c
af31a6a
9c8c525
1c0fdd8
09777b0
4573261
1ead736
5f71202
d23039e
0958d9f
db59292
8b5e6e8
99dd9e7
9eadb87
bb464fb
c709b62
5a824b4
3ae4aef
d18ba47
09ddbb4
059db57
16d1fd0
9d62564
e3fa364
54c5c18
9bf9bb2
5fc6195
14a0e82
7650fa2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,22 @@ | ||
//! Command-line options. | ||
use std::{collections::HashMap, path::PathBuf, str::FromStr}; | ||
|
||
use clap::{arg, Parser}; | ||
use rattler_conda_types::{ | ||
utils::url_with_trailing_slash::UrlWithTrailingSlash, NamedChannelOrUrl, Platform, | ||
}; | ||
use rattler_conda_types::utils::url_with_trailing_slash::UrlWithTrailingSlash; | ||
use rattler_conda_types::{NamedChannelOrUrl, Platform}; | ||
use rattler_networking::mirror_middleware; | ||
use rattler_networking::AuthenticationStorage; | ||
#[cfg(feature = "s3")] | ||
use rattler_networking::s3_middleware; | ||
use rattler_networking::{mirror_middleware, AuthenticationStorage}; | ||
use rattler_s3::clap::S3CredentialsOpts; | ||
#[cfg(feature = "s3")] | ||
use rattler_s3::{S3AddressingStyle, S3Credentials}; | ||
use rattler_solve::ChannelPriority; | ||
use std::{collections::HashMap, path::PathBuf, str::FromStr}; | ||
use tracing::warn; | ||
use url::Url; | ||
|
||
/// The configuration type for rattler-build - just extends rattler / pixi | ||
/// config and can load the same TOML files. | ||
#[cfg(feature = "s3")] | ||
use rattler_networking::s3_middleware; | ||
|
||
/// The configuration type for rattler-build - just extends rattler / pixi config and can load the same TOML files. | ||
pub type Config = rattler_config::config::ConfigBase<()>; | ||
|
||
/// Container for `rattler_solver::ChannelPriority` so that it can be parsed | ||
|
@@ -99,8 +102,7 @@ impl CommonData { | |
allow_insecure_host: Option<Vec<String>>, | ||
) -> Self { | ||
// mirror config | ||
// todo: this is a duplicate in pixi and pixi-pack: do it like in | ||
// `compute_s3_config` | ||
// todo: this is a duplicate in pixi and pixi-pack: do it like in `compute_s3_config` | ||
let mut mirror_config = HashMap::new(); | ||
tracing::debug!("Using mirrors: {:?}", config.mirrors); | ||
|
||
|
@@ -158,13 +160,16 @@ impl CommonData { | |
/// Upload options. | ||
#[derive(Parser, Debug)] | ||
pub struct UploadOpts { | ||
/// The host + channel (optional if the server type is provided) | ||
pub host: Option<Url>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are clap arguments here to make sure that if host is provide that channel must also be provide. Or that it conflicts with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, Bas! Nice catch! I mean the hostURL should consider "host" + "channel", and in |
||
|
||
/// The package file to upload | ||
#[arg(global = true, required = false)] | ||
pub package_files: Vec<PathBuf>, | ||
|
||
/// The server type | ||
//// The server type (optional if host is provided) | ||
#[clap(subcommand)] | ||
pub server_type: ServerType, | ||
pub server_type: Option<ServerType>, | ||
|
||
/// Common options. | ||
#[clap(flatten)] | ||
|
@@ -426,19 +431,102 @@ fn parse_s3_url(value: &str) -> Result<Url, String> { | |
#[cfg(feature = "s3")] | ||
#[derive(Clone, Debug, PartialEq, Parser)] | ||
pub struct S3Opts { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can reuse the S3CredentialsOpts from crates/rattler_s3/src/clap.rs here to get a consistent CLI. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do it! thanks for reminding! |
||
/// The channel URL in the S3 bucket to upload the package to, e.g., | ||
/// `s3://my-bucket/my-channel` | ||
/// The channel URL in the S3 bucket to upload the package to, e.g., `s3://my-bucket/my-channel` | ||
#[arg(short, long, env = "S3_CHANNEL", value_parser = parse_s3_url)] | ||
pub channel: Url, | ||
|
||
/// S3 credentials | ||
#[clap(flatten)] | ||
pub credentials: rattler_s3::clap::S3CredentialsOpts, | ||
pub s3_credentials: S3CredentialsOpts, | ||
|
||
/// Replace files if it already exists. | ||
#[arg(long)] | ||
/// S3 credentials (set programmatically, not via CLI) | ||
#[clap(skip)] | ||
pub credentials: Option<S3Credentials>, | ||
|
||
/// Replace files on conflict | ||
#[arg(long, short, env = "ANACONDA_FORCE")] | ||
pub force: bool, | ||
} | ||
|
||
#[cfg(feature = "s3")] | ||
#[derive(Debug)] | ||
#[allow(missing_docs)] | ||
pub struct S3Data { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of this struct? The name is quite ambigous. Also, doesnt the rattler_s3 crate provide similar functionality? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, Bas. S3Data was migrated before we had |
||
pub channel: Url, | ||
pub endpoint_url: Url, | ||
pub region: Option<String>, | ||
pub force_path_style: bool, | ||
pub credentials: Option<S3Credentials>, | ||
pub force: bool, | ||
} | ||
|
||
#[cfg(feature = "s3")] | ||
impl From<S3Opts> for S3Data { | ||
fn from(value: S3Opts) -> Self { | ||
let addressing_style = value.s3_credentials.addressing_style.into(); | ||
let force_path_style = matches!(addressing_style, S3AddressingStyle::Path); | ||
|
||
let credentials: Option<S3Credentials> = | ||
if let (Some(access_key_id), Some(secret_access_key)) = ( | ||
value.s3_credentials.access_key_id.clone(), | ||
value.s3_credentials.secret_access_key.clone(), | ||
) { | ||
Some(S3Credentials { | ||
endpoint_url: value | ||
.s3_credentials | ||
.endpoint_url | ||
.clone() | ||
.expect("endpoint_url is required"), | ||
region: value | ||
.s3_credentials | ||
.region | ||
.clone() | ||
.expect("region is required"), | ||
addressing_style, | ||
access_key_id: Some(access_key_id), | ||
secret_access_key: Some(secret_access_key), | ||
session_token: value.s3_credentials.session_token.clone(), | ||
}) | ||
} else { | ||
value.credentials | ||
}; | ||
|
||
Self { | ||
channel: value.channel, | ||
endpoint_url: value | ||
.s3_credentials | ||
.endpoint_url | ||
.expect("endpoint_url is required"), | ||
region: value.s3_credentials.region, | ||
force_path_style, | ||
credentials, | ||
force: value.force, | ||
} | ||
} | ||
} | ||
|
||
#[cfg(feature = "s3")] | ||
impl S3Data { | ||
/// Create a new instance of `S3Data` | ||
pub fn new( | ||
channel: Url, | ||
endpoint_url: Url, | ||
region: Option<String>, | ||
force_path_style: bool, | ||
credentials: Option<S3Credentials>, | ||
force: bool, | ||
) -> Self { | ||
Self { | ||
channel, | ||
endpoint_url, | ||
region, | ||
force_path_style, | ||
credentials, | ||
force, | ||
} | ||
} | ||
} | ||
|
||
#[derive(Debug)] | ||
#[allow(missing_docs)] | ||
pub struct AnacondaData { | ||
|
@@ -607,8 +695,7 @@ pub struct DebugOpts { | |
#[clap(flatten)] | ||
pub common: CommonOpts, | ||
|
||
/// Name of the specific output to debug (only required when a recipe has | ||
/// multiple outputs) | ||
/// Name of the specific output to debug (only required when a recipe has multiple outputs) | ||
#[arg(long, help = "Name of the specific output to debug")] | ||
pub output_name: Option<String>, | ||
} | ||
|
@@ -635,8 +722,8 @@ pub struct DebugData { | |
} | ||
|
||
impl DebugData { | ||
/// Generate a new `TestData` struct from `TestOpts` and an optional pixi | ||
/// config. `TestOpts` have higher priority than the pixi config. | ||
/// Generate a new `TestData` struct from `TestOpts` and an optional pixi config. | ||
/// `TestOpts` have higher priority than the pixi config. | ||
pub fn from_opts_and_config(opts: DebugOpts, config: Option<Config>) -> Self { | ||
Self { | ||
recipe_path: opts.recipe, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we fill in e.g.. API key from the args here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, Wolf. There're three things I need to explain.
According to pixi doc, upload doesn't have
api_key
arg. https://pixi.sh/latest/reference/cli/pixi/upload/#descriptionThe common args was designed like this(not having
api_key
) option.Only when you specify the subcommand as "prefix", can you pass the arg
api_key
. But in our case, we upload package through URL(prefix pattern),api_key
is not allowed to pass through the args. Users must pass the token by setting them locally.