Skip to content

Conversation

@pavelzw
Copy link
Member

@pavelzw pavelzw commented Mar 3, 2025

Description

xref #1096

@pavelzw pavelzw changed the title feat: Add support for repodata patching in rattler-index feat: Add support for repodata patching in rattler-index, fix silent failures Mar 5, 2025
))
})
.collect::<Vec<_>>();
try_join_all(tasks).await?;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was silently failing before because it only checked whether the join handles were okay but not whether the actual return values of the tasks were

Comment on lines +314 to +318
pb.abandon_with_message(format!(
"{} {}",
console::style("Failed to index").red(),
console::style(subdir.as_str()).dim()
));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not perfect as the progress bar can get overwritten by other tasks that are still executed after this failure. haven't seen a nice way to avoid this behavior

image

i think we can keep it for now like this since the error messages are printed anyway and i expect this script to be executed in non-interactive environments anyway most of the time

/// The maximum number of packages to process in-memory simultaneously.
/// This is necessary to limit memory usage when indexing large channels.
#[arg(long, default_value = "128", global = true)]
#[arg(long, default_value = "32", global = true)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when testing it out with 128 on aws s3, i regularly got errors that weren't there when limiting ourselves to 32 maximum concurrent downloads

@pavelzw
Copy link
Member Author

pavelzw commented Mar 5, 2025

the failing JS tests don't look related, right?

@pavelzw pavelzw marked this pull request as ready for review March 5, 2025 01:22
@pavelzw
Copy link
Member Author

pavelzw commented Mar 5, 2025

@wolfv @baszalmstra this is ready for review

@wolfv
Copy link
Contributor

wolfv commented Mar 5, 2025

Just FYI, a PatchInstructions v2 file will be landing soon: conda/ceps#108

@pavelzw
Copy link
Member Author

pavelzw commented Mar 5, 2025

Will keep this in mind when implementing sharding in rattler-index 👍🏻

@wolfv wolfv merged commit c69b2d1 into conda:main Mar 10, 2025
16 checks passed
@pavelzw pavelzw deleted the repodata-patching branch March 10, 2025 11:11
@baszalmstra baszalmstra mentioned this pull request Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants