Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Introduce download-retry and download-retry-wait settings in ZConfig #400

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Introduce download-retry and download-retry-wait settings in ZConfig #400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions zypp-core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ SET( zypp_toplevel_SRCS
)
INSTALL( FILES ${zypp_toplevel_headers} DESTINATION "${INCLUDE_INSTALL_DIR}/zypp-core" )

SET( zypp_base_private_HEADERS
base/private/configoption_p.h
)
Copy link
Member

Choose a reason for hiding this comment

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

Ripping out the struct Options, placing them in a new header file and adding the header file to CMake schoul go into a separate commit. That's pure cleanup and not related to any fix.


SET( zypp_base_HEADERS
base/DefaultIntegral
base/defaultintegral.h
Expand Down Expand Up @@ -333,6 +337,7 @@ SET ( zypp_HEADERS
${zypp_toplevel_headers}
${zyppng_async_HEADERS}
${zypp_base_HEADERS}
${zypp_base_private_HEADERS}
${zypp_fs_HEADERS}
${zypp_ui_HEADERS}
${zypp_url_HEADERS}
Expand Down
74 changes: 74 additions & 0 deletions zypp-core/base/private/configoption_p.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#ifndef ZYPP_CORE_BASE_PRIVATE_CONFIGOPTION_P_H
#define ZYPP_CORE_BASE_PRIVATE_CONFIGOPTION_P_H

namespace zypp
{

/** Mutable option. */
template<class Tp>
struct Option
{
typedef Tp value_type;

/** No default ctor, explicit initialisation! */
Option( value_type initial_r )
: _val( std::move(initial_r) )
{}

Option & operator=( value_type newval_r )
{ set( std::move(newval_r) ); return *this; }

/** Get the value. */
const value_type & get() const
{ return _val; }

/** Autoconversion to value_type. */
operator const value_type &() const
{ return _val; }

/** Set a new value. */
void set( value_type newval_r )
{ _val = std::move(newval_r); }

private:
value_type _val;
};

/** Mutable option with initial value also remembering a config value. */
template<class Tp>
struct DefaultOption : public Option<Tp>
{
typedef Tp value_type;
typedef Option<Tp> option_type;

explicit DefaultOption( value_type initial_r )
: Option<Tp>( initial_r )
, _default( std::move(initial_r) )
{}

DefaultOption & operator=( value_type newval_r )
{ this->set( std::move(newval_r) ); return *this; }

/** Reset value to the current default. */
void restoreToDefault()
{ this->set( _default.get() ); }

/** Reset value to a new default. */
void restoreToDefault( value_type newval_r )
{ setDefault( std::move(newval_r) ); restoreToDefault(); }

/** Get the current default value. */
const value_type & getDefault() const
{ return _default.get(); }

/** Set a new default value. */
void setDefault( value_type newval_r )
{ _default.set( std::move(newval_r) ); }

private:
option_type _default;
};

}

#endif // ZYPP_CORE_BASE_PRIVATE_CONFIGOPTION_P_H
74 changes: 59 additions & 15 deletions zypp-media/mediaconfig.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,40 @@
#include "mediaconfig.h"
#include <zypp-core/Pathname.h>
#include <zypp-core/base/String.h>
#include <zypp-core/base/private/configoption_p.h>

namespace zypp {

class MediaConfigPrivate {
public:

MediaConfigPrivate()
: download_max_concurrent_connections( 5 )
, download_min_download_speed ( 0 )
, download_max_download_speed ( 0 )
, download_max_silent_tries ( 5 )
, download_transfer_timeout ( 180 )
{ }
MediaConfigPrivate(){ }


void set_download_retry_wait_time( long val, bool overrideDefault = false ) {
if ( val < 0 ) val = 0;
else if ( val > 3600 ) val = 3600;
if ( overrideDefault ) download_retry_wait_time.setDefault( val );
else download_retry_wait_time = val;
}

void set_download_max_silent_tries( long val, bool overrideDefault = false ) {
if ( val < 0 ) val = 0;
else if ( val > 10 ) val = 10;
Copy link
Member

Choose a reason for hiding this comment

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

If you introduce new limit's, then it must be documented. The accessor in ZConfig.h schould mention it. If it is also exposed in zypp.conf we should think twice whether we need to introduce a restriction (an break existing behavior). If so, it must be documented in the zypp.conf template we install in the system and the zypper man page must be checked, whether it mentions the value. And we need a good explanation why?.


if ( overrideDefault ) download_max_silent_tries.setDefault( val );
else download_max_silent_tries = val;
}

Pathname credentials_global_dir_path;
Pathname credentials_global_file_path;

int download_max_concurrent_connections;
int download_min_download_speed;
int download_max_download_speed;
int download_max_silent_tries;
int download_transfer_timeout;
int download_max_concurrent_connections = 5;
int download_min_download_speed = 0;
int download_max_download_speed = 0;
DefaultOption<long> download_max_silent_tries {1};
DefaultOption<long> download_retry_wait_time {0};
int download_transfer_timeout = 180;
};

MediaConfig::MediaConfig() : d_ptr( new MediaConfigPrivate() )
Expand Down Expand Up @@ -70,14 +82,21 @@ namespace zypp {
return true;

} else if ( entry == "download.max_silent_tries" ) {
str::strtonum(value, d->download_max_silent_tries);
long val;
str::strtonum(value, val);
d->set_download_max_silent_tries( val, true );
return true;

} else if ( entry == "download.transfer_timeout" ) {
str::strtonum(value, d->download_transfer_timeout);
if ( d->download_transfer_timeout < 0 ) d->download_transfer_timeout = 0;
else if ( d->download_transfer_timeout > 3600 ) d->download_transfer_timeout = 3600;
return true;
} else if ( entry == "download.retry_wait_time" ) {
long val;
str::strtonum(value, val);
d->set_download_retry_wait_time( val, true );
return true;
}
}
return false;
Expand Down Expand Up @@ -109,10 +128,35 @@ namespace zypp {
long MediaConfig::download_max_silent_tries() const
{ return d_func()->download_max_silent_tries; }

void MediaConfig::set_download_max_silent_tries( long val )
{
Z_D();
d->set_download_max_silent_tries( val );
}

void MediaConfig::reset_download_max_silent_tries ()
{
Z_D();
d->download_max_silent_tries.restoreToDefault();
}

long MediaConfig::download_retry_wait_time() const
{ return d_func()->download_retry_wait_time; }

void MediaConfig::set_download_retry_wait_time ( long val )
{
Z_D();
d->set_download_retry_wait_time( val );
}

void MediaConfig::reset_download_retry_wait_time ()
{
Z_D();
d->download_retry_wait_time.restoreToDefault();
}

long MediaConfig::download_transfer_timeout() const
{ return d_func()->download_transfer_timeout; }

ZYPP_IMPL_PRIVATE(MediaConfig)
}


19 changes: 19 additions & 0 deletions zypp-media/mediaconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,25 @@ namespace zypp {
*/
long download_max_silent_tries() const;

void set_download_max_silent_tries( long val );

/*!
* Reset the download max tries to the configured default
*/
void reset_download_max_silent_tries ();

/*!
* How long should zypp wait before attempting a retry of a failed download
*/
long download_retry_wait_time() const;

void set_download_retry_wait_time ( long val );

/*!
* Reset the download retry time to the configured default
*/
void reset_download_retry_wait_time ();

/*!
* Maximum time in seconds that you allow a transfer operation to take.
*/
Expand Down
81 changes: 16 additions & 65 deletions zypp/ZConfig.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ extern "C"
#include <zypp/base/LogTools.h>
#include <zypp/base/IOStream.h>
#include <zypp-core/base/InputStream>
#include <zypp-core/base/private/configoption_p.h>
#include <zypp/base/String.h>
#include <zypp/base/Regex.h>

Expand Down Expand Up @@ -238,71 +239,6 @@ namespace zypp
} // namespace zypp
///////////////////////////////////////////////////////////////////

/** Mutable option. */
template<class Tp>
struct Option
{
typedef Tp value_type;

/** No default ctor, explicit initialisation! */
Option( value_type initial_r )
: _val( std::move(initial_r) )
{}

Option & operator=( value_type newval_r )
{ set( std::move(newval_r) ); return *this; }

/** Get the value. */
const value_type & get() const
{ return _val; }

/** Autoconversion to value_type. */
operator const value_type &() const
{ return _val; }

/** Set a new value. */
void set( value_type newval_r )
{ _val = std::move(newval_r); }

private:
value_type _val;
};

/** Mutable option with initial value also remembering a config value. */
template<class Tp>
struct DefaultOption : public Option<Tp>
{
typedef Tp value_type;
typedef Option<Tp> option_type;

explicit DefaultOption( value_type initial_r )
: Option<Tp>( initial_r )
, _default( std::move(initial_r) )
{}

DefaultOption & operator=( value_type newval_r )
{ this->set( std::move(newval_r) ); return *this; }

/** Reset value to the current default. */
void restoreToDefault()
{ this->set( _default.get() ); }

/** Reset value to a new default. */
void restoreToDefault( value_type newval_r )
{ setDefault( std::move(newval_r) ); restoreToDefault(); }

/** Get the current default value. */
const value_type & getDefault() const
{ return _default.get(); }

/** Set a new default value. */
void setDefault( value_type newval_r )
{ _default.set( std::move(newval_r) ); }

private:
option_type _default;
};

///////////////////////////////////////////////////////////////////
//
// CLASS NAME : ZConfig::Impl
Expand Down Expand Up @@ -1086,9 +1022,24 @@ namespace zypp
long ZConfig::download_max_silent_tries() const
{ return _pimpl->_mediaConf.download_max_silent_tries(); }

void ZConfig::set_download_max_silent_tries( long val )
{ _pimpl->_mediaConf.set_download_max_silent_tries(val); }

void ZConfig::reset_download_max_silent_tries( )
{ _pimpl->_mediaConf.reset_download_max_silent_tries(); }

long ZConfig::download_transfer_timeout() const
{ return _pimpl->_mediaConf.download_transfer_timeout(); }

long ZConfig::download_retry_wait_time() const
{ return _pimpl->_mediaConf.download_retry_wait_time(); }

void ZConfig::set_download_retry_wait_time ( long val )
{ _pimpl->_mediaConf.set_download_retry_wait_time(val); }

void ZConfig::reset_download_retry_wait_time ()
{ _pimpl->_mediaConf.reset_download_retry_wait_time(); }

Pathname ZConfig::download_mediaMountdir() const { return _pimpl->download_mediaMountdir; }
void ZConfig::set_download_mediaMountdir( Pathname newval_r ) { _pimpl->download_mediaMountdir.set( std::move(newval_r) ); }
void ZConfig::set_default_download_mediaMountdir() { _pimpl->download_mediaMountdir.restoreToDefault(); }
Expand Down
11 changes: 11 additions & 0 deletions zypp/ZConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,17 @@ namespace zypp
*/
long download_max_silent_tries() const;

void set_download_max_silent_tries( long val );
void reset_download_max_silent_tries( );

/*!
* How long should zypp wait before attempting a retry of a failed download
*/
long download_retry_wait_time() const;

void set_download_retry_wait_time ( long val );
void reset_download_retry_wait_time ();

/**
* Maximum time in seconds that you allow a transfer operation to take.
*/
Expand Down