Skip to content

Commit

Permalink
try to fix more mirror things
Browse files Browse the repository at this point in the history
  • Loading branch information
@wolfv
wolfv committed Jan 19, 2023
1 parent 4854793 commit 41ad002
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 82 deletions.
4 changes: 4 additions & 0 deletions include/powerloader/context.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@ namespace powerloader
class mirror_map_type : private mirror_map_base
{
public:
using mirror_map_base::begin;
using mirror_map_base::clear;
using mirror_map_base::empty;
using mirror_map_base::end;
using mirror_map_base::mirror_map_base;
using mirror_map_base::size;

// Get a list of unique mirorrs if existing for the provided host name, or an empty list
// otherwise.
Expand Down
14 changes: 3 additions & 11 deletions include/powerloader/download_target.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ namespace powerloader
void set_cache_options(const CacheControl& cache_control);
void add_handle_options(CURLHandle& handle);

bool has_complete_url() const;
bool validate_checksum(const fs::path& path);
bool already_downloaded();

Expand Down Expand Up @@ -113,12 +112,6 @@ namespace powerloader
m_base_url.clear();
}


const std::string& complete_url() const noexcept
{
return m_complete_url;
}

const std::string& path() const noexcept
{
return m_path;
Expand Down Expand Up @@ -167,22 +160,22 @@ namespace powerloader
return m_range;
}

bool is_zchunck() const noexcept
bool is_zchunk() const noexcept
{
return m_is_zchunk;
}

const zck_target& zck() const
{
if (!is_zchunck()) // TODO: REVIEW: should this be an assert?
if (!is_zchunk()) // TODO: REVIEW: should this be an assert?
throw std::invalid_argument("attempted to access zchunk data but there is none");
return *m_p_zck;
}

// TODO: ownership/access issue: mostly modified outside
zck_target& zck()
{
if (!is_zchunck()) // TODO: REVIEW: should this be an assert?
if (!is_zchunk()) // TODO: REVIEW: should this be an assert?
throw std::invalid_argument("attempted to access zchunk data but there is none");
return *m_p_zck;
}
Expand Down Expand Up @@ -288,7 +281,6 @@ namespace powerloader
bool m_no_cache = false;
bool m_head_only = false;

std::string m_complete_url;
std::string m_path;
std::string m_base_url;
std::unique_ptr<FileIO> m_outfile;
Expand Down
30 changes: 14 additions & 16 deletions src/download_target.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,8 @@ namespace powerloader
, m_base_url(base_url)
, m_destination_path(destination)
{
if (path.find("://") != std::string::npos)
{
m_complete_url = path;
}
else if (base_url.find("://") != std::string::npos)
{
m_complete_url = join_url(base_url, path);
}
spdlog::warn(
"DownloadTarget::DownloadTarget: {}, {}, {}", path, base_url, destination.string());

#if WITH_ZCHUNK
if (m_is_zchunk)
Expand Down Expand Up @@ -59,10 +53,12 @@ namespace powerloader
const std::string mirror_url = uh.url_without_path();
const fs::path dst = destination_path.empty() ? fs::path{ rsplit(path, "/", 1).back() }
: destination_path;
spdlog::warn("SETTING MIRRORR >>>> {}, {}", host, mirror_url);

ctx.mirror_map.create_unique_mirror<Mirror>(host, ctx, mirror_url);
ctx.mirror_map.create_unique_mirror<Mirror>(mirror_url, ctx, mirror_url);

return std::make_shared<DownloadTarget>(path.substr(1, std::string::npos), host, dst);
return std::make_shared<DownloadTarget>(
path.substr(1, std::string::npos), mirror_url, dst);
}
else
{
Expand All @@ -73,7 +69,14 @@ namespace powerloader
}
const auto mirror = hostname_override ? hostname_override.value() : parts[0];
const auto path = parts[1];

if (!ctx.mirror_map.has_mirrors(mirror))
{
throw std::runtime_error("Mirror " + mirror + " not found");
}
else
{
spdlog::warn("Mirror {} already exists", mirror);
}
fs::path dst = destination_path.empty() ? fs::path{ rsplit(path, "/", 1).back() }
: destination_path;

Expand All @@ -86,11 +89,6 @@ namespace powerloader

DownloadTarget::~DownloadTarget() = default;

bool DownloadTarget::has_complete_url() const
{
return !m_complete_url.empty();
}

bool DownloadTarget::validate_checksum(const fs::path& path)
{
if (m_checksums.empty())
Expand Down
75 changes: 31 additions & 44 deletions src/downloader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ namespace powerloader
target->headercb_interrupt_reason()) });
}
#ifdef WITH_ZCHUNK
else if (target->range_fail())
else if (target->range_fail() && target->target().is_zchunk())
{
zckRange* range = zck_dl_get_range(target->target().zck().zck_dl);
int range_count = zck_get_range_count(range);
Expand All @@ -115,7 +115,7 @@ namespace powerloader
target->mirror()->stats().max_ranges);
}
}
else if (target->target().zck().zck_dl != nullptr
else if (target->target().is_zchunk() && target->target().zck().zck_dl != nullptr
&& zck_is_error(zck_dl_get_zck(target->target().zck().zck_dl)) > 0)
{
zckCtx* zck = zck_dl_get_zck(target->target().zck().zck_dl);
Expand Down Expand Up @@ -263,7 +263,7 @@ namespace powerloader
}

if (mirrors_iterated == 0 && mirror->protocol() == Protocol::kFTP
&& target->target().is_zchunck())
&& target->target().is_zchunk())
{
continue;
}
Expand Down Expand Up @@ -302,7 +302,7 @@ namespace powerloader
return tl::unexpected(DownloaderError(
{ ErrorLevel::FATAL,
ErrorCode::PD_NOURL,
fmt::format("No suitable mirror found for {}", target->target().complete_url()) }));
fmt::format("No suitable mirror found for {}", target->target().base_url()) }));
}

// Select next target
Expand All @@ -317,12 +317,9 @@ namespace powerloader
if (target->state() != DownloadState::kWAITING)
continue;

// Determine if path is a complete URL
bool complete_url_in_path = target->target().has_complete_url();

bool have_mirrors = !target->mirrors().empty();
// Sanity check
if (target->target().base_url().empty() && !have_mirrors && !complete_url_in_path)
if (target->target().base_url().empty() && !have_mirrors)
{
// Used relative path with empty internal mirrorlist and no basepath specified!
return tl::unexpected(DownloaderError{
Expand All @@ -332,43 +329,36 @@ namespace powerloader
}

// Prepare full target URL
if (complete_url_in_path)
auto res = select_suitable_mirror(target);
if (!res)
{
full_url = target->target().complete_url();
// TODO: review this: why is the callback called without changing the state
// of the target? (see Target::set_failed() for example).
target->call_end_callback(TransferStatus::kERROR);
return tl::unexpected(res.error());
}
else
{
// Find a suitable mirror
auto res = select_suitable_mirror(target);
if (!res)
{
// TODO: review this: why is the callback called without changing the state
// of the target? (see Target::set_failed() for example).
target->call_end_callback(TransferStatus::kERROR);
return tl::unexpected(res.error());
}

mirror = res.value();
mirror = res.value();

assert(mirror);
assert(mirror);

// TODO: create a `name()` or similar function
spdlog::info("Selected mirror: {}", mirror->url());
if (mirror && !mirror->needs_preparation(target))
{
full_url = mirror->format_url(target);
target->change_mirror(mirror);
}
else
// TODO: create a `name()` or similar function
spdlog::info("Selected mirror: {}", mirror->url());
if (mirror && !mirror->needs_preparation(target))
{
full_url = mirror->format_url(target);
target->change_mirror(mirror);
}
else
{
// No free mirror
if (!mirror->needs_preparation(target))
{
// No free mirror
if (!mirror->needs_preparation(target))
{
spdlog::info("Currently there is no free mirror for {}",
target->target().path());
}
spdlog::info("Currently there is no free mirror for {}",
target->target().path());
}
}
// }

// If LRO_OFFLINE is specified, check if the obtained full_url is local or not
// This condition should never be true for a full_url built from a mirror, because
Expand Down Expand Up @@ -579,7 +569,6 @@ namespace powerloader
if (!result)
{
// int complete_url_in_path = strstr(target->target().path(), "://") ? 1 : 0;
int complete_url_in_path = false;

bool retry = false;

Expand Down Expand Up @@ -629,12 +618,11 @@ namespace powerloader
current_target->lower_mirror_parallel_connections();
}

// TODO FIX remove complete url stuff
// complete_url_in_path and target->base_url() doesn't have an
// alternatives like using mirrors, therefore they are handled
// differently
std::string complete_url_or_base_url
= complete_url_in_path ? current_target->target().path()
: current_target->target().base_url();
std::string complete_url_or_base_url = current_target->target().path();
if (can_retry_download(static_cast<int>(current_target->retries()),
complete_url_or_base_url))
{
Expand Down Expand Up @@ -664,8 +652,7 @@ namespace powerloader
if (!retry)
{
// No more mirrors to try or base_url used or fatal error
spdlog::error("Retries exceeded for {}",
current_target->target().complete_url());
spdlog::error("Retries exceeded for {}", current_target->target().base_url());

assert(!result);
const CbReturnCode rc = current_target->set_failed(result.error());
Expand Down Expand Up @@ -829,7 +816,7 @@ namespace powerloader

for (auto* dl_target : dl_targets)
{
if (dl_target->is_zchunck())
if (dl_target->is_zchunk())
{
fs::path p = dl_target->destination_path();
try
Expand Down
2 changes: 1 addition & 1 deletion src/mirror.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ namespace powerloader

std::string Mirror::format_url(Target* target) const
{
return fmt::format("{}/{}", m_url, target->target().path());
return join_url(m_url, target->target().path());
}

/** Sort mirrors. Penalize the error ones.
Expand Down
1 change: 0 additions & 1 deletion src/python/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ PYBIND11_MODULE(pypowerloader, m)

py::class_<DownloadTarget, std::shared_ptr<DownloadTarget>>(m, "DownloadTarget")
.def(py::init<const std::string&, const std::string&, const fs::path&>())
.def_property_readonly("complete_url", &DownloadTarget::complete_url)
.def_property("progress_callback",
&DownloadTarget::progress_callback,
&DownloadTarget::set_progress_callback);
Expand Down
22 changes: 13 additions & 9 deletions src/target.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ namespace powerloader
, m_mirrors(std::move(mirrors))
, m_ctx(ctx)
{
for (auto& m : m_mirrors)
{
spdlog::warn("Mirror: {} for target {}", m->url(), m_target->path());
}
}

Target::~Target()
Expand All @@ -30,7 +34,7 @@ namespace powerloader
bool Target::zck_running() const
{
#ifdef WITH_ZCHUNK
return m_target->is_zchunck() && m_zck_state != ZckState::kFINISHED;
return m_target->is_zchunk() && m_zck_state != ZckState::kFINISHED;
#else
return false;
#endif
Expand Down Expand Up @@ -186,7 +190,7 @@ namespace powerloader
}

#ifdef WITH_ZCHUNK
if (target->target().is_zchunck() && !target->m_range_fail && target->m_mirror
if (target->target().is_zchunk() && !target->m_range_fail && target->m_mirror
&& target->m_mirror->protocol() == Protocol::kHTTP)
return zckheadercb(buffer, size, nitems, self);
#endif /* WITH_ZCHUNK */
Expand Down Expand Up @@ -326,7 +330,7 @@ namespace powerloader
std::size_t cur_written_expected = nitems, cur_written;

#ifdef WITH_ZCHUNK
if (self->m_target->is_zchunck() && !self->m_range_fail && self->m_mirror
if (self->m_target->is_zchunk() && !self->m_range_fail && self->m_mirror
&& self->m_mirror->protocol() == Protocol::kHTTP)
{
return zckwritecb(buffer, size, nitems, self);
Expand Down Expand Up @@ -439,7 +443,7 @@ namespace powerloader
}

#ifdef WITH_ZCHUNK
if (target->m_target->is_zchunck())
if (target->m_target->is_zchunk())
{
total_to_download = target->m_target->zck().total_to_download;
now_downloaded = now_downloaded + target->m_target->zck().downloaded;
Expand Down Expand Up @@ -553,14 +557,14 @@ namespace powerloader
}
// Prepare FILE
#ifdef WITH_ZCHUNK
if (!m_target->is_zchunck())
if (!m_target->is_zchunk())
{
#endif
this->open_target_file();
#ifdef WITH_ZCHUNK
}
// If file is zchunk, prep it
if (m_target->is_zchunck())
if (m_target->is_zchunk())
{
if (!m_target->outfile())
{
Expand Down Expand Up @@ -712,7 +716,7 @@ namespace powerloader
tl::expected<void, DownloaderError> Target::finish_transfer(const std::string& effective_url)
{
#ifdef WITH_ZCHUNK
if (m_target->is_zchunck())
if (m_target->is_zchunk())
{
if (m_zck_state == ZckState::kHEADER_LEAD)
{
Expand Down Expand Up @@ -865,7 +869,7 @@ namespace powerloader
m_retries++;

#ifdef WITH_ZCHUNK
if (!m_target->is_zchunck() || m_zck_state == ZckState::kHEADER)
if (!m_target->is_zchunk() || m_zck_state == ZckState::kHEADER)
{
#endif
// Truncate file - remove downloaded garbage (error html page etc.)
Expand All @@ -882,7 +886,7 @@ namespace powerloader
void Target::finalize_transfer(const std::string& effective_url)
{
#ifdef WITH_ZCHUNK
if (m_target->is_zchunck() && m_zck_state != ZckState::kFINISHED)
if (m_target->is_zchunk() && m_zck_state != ZckState::kFINISHED)
{
m_state = DownloadState::kWAITING;
if (m_mirror)
Expand Down

0 comments on commit 41ad002

Please sign in to comment.