Message ID | 20180919114541.17670-1-giulio.benetti@micronovasrl.com |
---|---|
State | Rejected |
Headers | show |
Series | [buildroot-test] autobuild-run: add --flush-downloads argument to prevent false NOK due to not enough space on hard drive | expand |
Tested on "Micronova srl Server" Il 19/09/2018 13:45, Giulio Benetti ha scritto: > Some server/pc can have little hard drive, but high speed internet > connection. If disk is little, autobuild-run could generate some false > NOK reporting: > Fatal error: ...: No space left on device > > Add --flush-downloads argument to autobuild-run to delete entire > instance-*/dl/ folder everytime it creates a new instance. > If active this will lead to re-download all files needed by an instance > in its dl/ folder when created. > As the other options, flush-downloads can be listed in .conf file like > this: > flush-downloads = yes > > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com> > --- > scripts/autobuild-run | 36 +++++++++++++++++++++++------------- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/scripts/autobuild-run b/scripts/autobuild-run > index 3d2e99a..2682239 100755 > --- a/scripts/autobuild-run > +++ b/scripts/autobuild-run > @@ -106,6 +106,7 @@ Options: > --toolchains-csv CSVFILE Toolchain configuration file > -r, --repo URL URL of Buildroot repository to clone > Defaults to %(--repo)s > + --flush-downloads Delete dl/ folder every time new instance starts > > Format of the configuration file: > > @@ -301,18 +302,26 @@ def prepare_build(**kwargs): > for i in f: > yield os.path.join(r, i) > > - # Remove 5 random files from the download directory. Removing > - # random files from the download directory allows to ensure we > - # regularly re-download files to check that their upstream > - # location is still correct. > - for i in range(0, 5): > - flist = list(find_files(dldir)) > - if not flist: > - break > - f = flist[randint(0, len(flist) - 1)] > - log_write(log, "INFO: removing %s from downloads" % > - os.path.relpath(f, dldir)) > - os.remove(f) > + flush_dl = kwargs['flush_dl'] > + > + # Check if we need to flush dl/ folder or keep it > + if flush_dl: > + log_write(log, "INFO: flush dl/ folder") > + # Remove entire dl/ folder > + shutil.rmtree(dldir) > + else: > + # Remove 5 random files from the download directory. Removing > + # random files from the download directory allows to ensure we > + # regularly re-download files to check that their upstream > + # location is still correct. > + for i in range(0, 5): > + flist = list(find_files(dldir)) > + if not flist: > + break > + f = flist[randint(0, len(flist) - 1)] > + log_write(log, "INFO: removing %s from downloads" % > + os.path.relpath(f, dldir)) > + os.remove(f) > > branch = get_branch() > log_write(log, "INFO: testing branch '%s'" % branch) > @@ -766,7 +775,8 @@ def main(): > repo = args['--repo'], > upload = upload, > buildpid = buildpid, > - debug = args['--debug'] > + debug = args['--debug'], > + flush_dl = args['--flush-downloads'] > )) > p.start() > processes.append(p) >
Hi Giulio, Sorry to be so late in responding to this... On 19/09/2018 13:45, Giulio Benetti wrote: > Some server/pc can have little hard drive, but high speed internet > connection. If disk is little, autobuild-run could generate some false > NOK reporting: > Fatal error: ...: No space left on device > > Add --flush-downloads argument to autobuild-run to delete entire > instance-*/dl/ folder everytime it creates a new instance. This probably doesn't *completely* solve the issue, because a single download can be very large as well. We discussed this at the hackaton and decided that it would be more appropriate to instead tweak the number of files to delete from the download directory. By setting that number very high, you can approximate the same result (except that the git repos will never be touched). The code changes for that would be a lot simpler we think, and we think it's more suitable as an option. Therefore, we've marked the patch as rejected in patchwork. Feel free to implement the suggestion above instead (or, of course, keep this as an out-of-tree patch). Regards, Arnout > If active this will lead to re-download all files needed by an instance > in its dl/ folder when created. > As the other options, flush-downloads can be listed in .conf file like > this: > flush-downloads = yes > > Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com> [snip]
Hi Arnout, Il 03/08/2019 14:46, Arnout Vandecappelle ha scritto: > Hi Giulio, > > Sorry to be so late in responding to this... No problem! > On 19/09/2018 13:45, Giulio Benetti wrote: >> Some server/pc can have little hard drive, but high speed internet >> connection. If disk is little, autobuild-run could generate some false >> NOK reporting: >> Fatal error: ...: No space left on device >> >> Add --flush-downloads argument to autobuild-run to delete entire >> instance-*/dl/ folder everytime it creates a new instance. > > This probably doesn't *completely* solve the issue, because a single download > can be very large as well. Right. > We discussed this at the hackaton and decided that it would be more appropriate > to instead tweak the number of files to delete from the download directory. By > setting that number very high, you can approximate the same result (except that > the git repos will never be touched). The code changes for that would be a lot > simpler we think, and we think it's more suitable as an option. Ok, I will do that. Kind regards
diff --git a/scripts/autobuild-run b/scripts/autobuild-run index 3d2e99a..2682239 100755 --- a/scripts/autobuild-run +++ b/scripts/autobuild-run @@ -106,6 +106,7 @@ Options: --toolchains-csv CSVFILE Toolchain configuration file -r, --repo URL URL of Buildroot repository to clone Defaults to %(--repo)s + --flush-downloads Delete dl/ folder every time new instance starts Format of the configuration file: @@ -301,18 +302,26 @@ def prepare_build(**kwargs): for i in f: yield os.path.join(r, i) - # Remove 5 random files from the download directory. Removing - # random files from the download directory allows to ensure we - # regularly re-download files to check that their upstream - # location is still correct. - for i in range(0, 5): - flist = list(find_files(dldir)) - if not flist: - break - f = flist[randint(0, len(flist) - 1)] - log_write(log, "INFO: removing %s from downloads" % - os.path.relpath(f, dldir)) - os.remove(f) + flush_dl = kwargs['flush_dl'] + + # Check if we need to flush dl/ folder or keep it + if flush_dl: + log_write(log, "INFO: flush dl/ folder") + # Remove entire dl/ folder + shutil.rmtree(dldir) + else: + # Remove 5 random files from the download directory. Removing + # random files from the download directory allows to ensure we + # regularly re-download files to check that their upstream + # location is still correct. + for i in range(0, 5): + flist = list(find_files(dldir)) + if not flist: + break + f = flist[randint(0, len(flist) - 1)] + log_write(log, "INFO: removing %s from downloads" % + os.path.relpath(f, dldir)) + os.remove(f) branch = get_branch() log_write(log, "INFO: testing branch '%s'" % branch) @@ -766,7 +775,8 @@ def main(): repo = args['--repo'], upload = upload, buildpid = buildpid, - debug = args['--debug'] + debug = args['--debug'], + flush_dl = args['--flush-downloads'] )) p.start() processes.append(p)
Some server/pc can have little hard drive, but high speed internet connection. If disk is little, autobuild-run could generate some false NOK reporting: Fatal error: ...: No space left on device Add --flush-downloads argument to autobuild-run to delete entire instance-*/dl/ folder everytime it creates a new instance. If active this will lead to re-download all files needed by an instance in its dl/ folder when created. As the other options, flush-downloads can be listed in .conf file like this: flush-downloads = yes Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com> --- scripts/autobuild-run | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-)