PR#1308: Allow specifying the temp dir when gathering from repo. - pungi

pungi

#1308 Allow specifying the temp dir when gathering from repo.

Closed 4 years ago by lsedlar. Opened 4 years ago by fepitre.

fepitre/pungi devel031219 into master

pungi-koji: handle passing gather tempdir to pungi-gather

Frédéric Pierret (fepitre) • 4 years ago

ca58e95

Allow specifying temp dir in pungi-gather

Frédéric Pierret (fepitre) • 4 years ago

26888c0

bin/pungi-gather

file modified

+13 -7

		`@@ -71,13 +71,16 @@`
		`metavar="[METHOD]",`
		`action="append",`
		`)`
		`+ group.add_argument(`
		`+ "--tempdir",`
		`+ metavar='PATH',`
		`+ help="path to temp dir (default: /tmp)",`
		`+ default='/tmp',`
		`+ )`
		`return parser`


		`- def main(persistdir, cachedir):`
		`- parser = get_parser()`
		`- ns = parser.parse_args()`
		`-`
		`+ def main(ns, persistdir, cachedir):`
		`dnf_conf = Conf(ns.arch)`
		`dnf_conf.persistdir = persistdir`
		`dnf_conf.cachedir = cachedir`
		`@@ -174,6 +177,9 @@`


		`if __name__ == "__main__":`
		`- with temp_dir(prefix='pungi_dnf_') as persistdir:`
		`- with temp_dir(prefix='pungi_dnf_cache_') as cachedir:`
		`- main(persistdir, cachedir)`
		`+ parser = get_parser()`
		`+ ns = parser.parse_args()`
		`+`
		`+ with temp_dir(dir=ns.tempdir, prefix='pungi_dnf_') as persistdir:`
		`+ with temp_dir(dir=ns.tempdir, prefix='pungi_dnf_cache_') as cachedir:`
		`+ main(ns, persistdir, cachedir)`

bin/pungi-koji

file modified

+13 -1

		`@@ -53,6 +53,12 @@`
		`help="reuse an existing compose directory (DANGEROUS!)",`
		`)`
		`parser.add_argument(`
		`+ "--gather-tempdir",`
		`+ metavar="PATH",`
		`+ help="path to gather temp dir (default: /tmp)",`
		`+ default="/tmp"`
		`+ )`
		`+ parser.add_argument(`
		`"--label",`
		`help="specify compose label (example: Snapshot-1.0); required for production composes"`
		`)`
		`@@ -193,6 +199,11 @@`
		`if not os.path.isdir(opts.compose_dir):`
		`abort("The compose directory does not exist or is not a directory: %s" % opts.compose_dir)`

		`+ if opts.gather_tempdir:`
		`+ opts.gather_tempdir = os.path.abspath(opts.gather_tempdir)`
		`+ if not os.path.isdir(opts.gather_tempdir):`
		`+ abort("The gather temp directory does not exist or is not a directory: %s" % opts.gather_tempdir)`
		`+`
		`opts.config = os.path.abspath(opts.config)`

		`create_latest_link = not opts.no_latest_link`
		`@@ -254,7 +265,8 @@`
		`koji_event=opts.koji_event,`
		`supported=opts.supported,`
		`logger=logger,`
		`- notifier=notifier)`
		`+ notifier=notifier,`
		`+ gather_tempdir=opts.gather_tempdir)`
		`notifier.compose = compose`
		`COMPOSE = compose`
		`run_compose(compose, create_latest_link=create_latest_link, latest_link_status=latest_link_status)`

pungi/compose.py

file modified

+2 -1

		`@@ -107,7 +107,7 @@`


		`class Compose(kobo.log.LoggingBase):`
		`- def __init__(self, conf, topdir, skip_phases=None, just_phases=None, old_composes=None, koji_event=None, supported=False, logger=None, notifier=None):`
		`+ def __init__(self, conf, topdir, skip_phases=None, just_phases=None, old_composes=None, koji_event=None, supported=False, logger=None, notifier=None, gather_tempdir=None):`
		`kobo.log.LoggingBase.__init__(self, logger)`
		`# TODO: check if minimal conf values are set`
		`self.conf = conf`
		`@@ -122,6 +122,7 @@`
		`self.old_composes = old_composes or []`
		`self.koji_event = koji_event or conf.get("koji_event")`
		`self.notifier = notifier`
		`+ self.gather_tempdir = gather_tempdir`

		`# path definitions`
		`self.paths = Paths(self)`

pungi/phases/pkgset/sources/source_repos.py

file modified

+1 -1

		`@@ -83,7 +83,7 @@`
		`selfhosting=True, fulltree=True, multilib_methods=["all"],`
		`nodownload=False, full_archlist=True, arch=arch,`
		`cache_dir=compose.paths.work.pungi_cache_dir(arch=arch),`
		`- profiler=profiler)`
		`+ profiler=profiler, tempdir=compose.gather_tempdir)`
		`if compose.conf['gather_backend'] == 'yum':`
		`cmd.append("--force")`

pungi/wrappers/pungi.py

file modified

+4 -1

		`@@ -155,7 +155,7 @@`

		`return cmd`

		`- def get_pungi_cmd_dnf(self, config, destdir, name, version=None, flavor=None, selfhosting=False, fulltree=False, greedy=None, nodeps=False, nodownload=True, full_archlist=False, arch=None, cache_dir=None, lookaside_repos=None, multilib_methods=None, profiler=False):`
		`+ def get_pungi_cmd_dnf(self, config, destdir, name, version=None, flavor=None, selfhosting=False, fulltree=False, greedy=None, nodeps=False, nodownload=True, full_archlist=False, arch=None, cache_dir=None, lookaside_repos=None, multilib_methods=None, profiler=False, tempdir=None):`
		`cmd = ["pungi-gather"]`

		`# path to a kickstart file`
		`@@ -192,6 +192,9 @@`
		`if profiler:`
		`cmd.append("--profiler")`

		`+ if tempdir:`
		`+ cmd.append("--tempdir=%s" % tempdir)`
		`+`
		`return cmd`

		`def parse_log(self, f):`

fepitre commented 4 years ago

That fix invalid cross-device link when hardlinking when /tmp is a separate filesystem.

This first version serves as an example of implementation for specifying gather temp dir. Opened to suggestions.

2 new commits added

pungi-koji: handle passing gather tempdir to pungi-gather
Allow specifying temp dir in pungi-gather

4 years ago

pretty please pagure-ci rebuild

4 years ago

fepitre commented 4 years ago

Looking closer to the code/doc, I'm seeing also that we can specify link_type to be a symlink. The primary concern here is to specifying temp directory not only due to this error.

lsedlar commented 4 years ago

Given that the amount of downloaded data can be really huge, it definitely makes sense to move it to another location that is less likely to run into space constraints. I'm not sure though if it needs to be customizable (especially since it adds a lot of plumbing to pass the value around).

In the get_pungi_cmd_dnf method there's a cache_dir unused argument that points to a path. Would that be usable instead of having to manage a location by yourself?

fepitre commented 4 years ago

Given that the amount of downloaded data can be really huge, it definitely makes sense to move it to another location that is less likely to run into space constraints. I'm not sure though if it needs to be customizable (especially since it adds a lot of plumbing to pass the value around).
In the get_pungi_cmd_dnf method there's a cache_dir unused argument that points to a path. Would that be usable instead of having to manage a location by yourself?

I think in view of what we discussed on the source repo solution, that would make sense to not use this PR. In case, unused cache_dir would be better in such case indeed. I suggest to ignore/close this PR while I'm doing an implementation of what we discussed.

Pull-Request has been closed by lsedlar

4 years ago

fepitre commented 4 years ago

@lsedlar: is it ok to merge only the commit for the tempdir for pungi-gather? In this case, I can open another PR with only the commit "Allow specifying temp dir in pungi-gather".

lsedlar commented 4 years ago

No need for new PR, I can cherry-pick the existing patch. It's on latest master now.

fepitre commented 4 years ago

Thank you.