PR#54: centos-sig lookaside cache support - centos/centpkg

centos / centpkg

#54 centos-sig lookaside cache support

Merged 2 years ago by lrossett. Opened 2 years ago by lrossett.

centos/ lrossett/centpkg lookaside-sig into develop

centos-sig lookaside cache support

Leonardo Rossetti • 2 years ago

f5ba26

src/bin/centpkg-sig

file modified

src/centpkg-sig.conf

file modified

+4 -1

		`@@ -1,7 +1,10 @@`
		`[centpkg-sig]`
		`lookaside = https://git.centos.org/sources`
		`lookasidehash = sha512`
		`- lookaside_cgi = https://git.centos.org/sources/upload.cgi`
		`+ lookaside_cgi = https://git.centos.org/sources/upload_sig.cgi`
		`+ # lookaside_cgi = https://git.centos.org/sources/upload.cgi`
		`+ lookaside_structure = hash`
		`+ # lookaside_structure = branch`
		`distgit_namespaced = True`
		`distgit_namespaces = rpms`
		`gitbaseurl = ssh://git@git.centos.org/%(repo)s.git`

src/centpkg/__init__.py

file modified

+7 -1

		`@@ -151,6 +151,11 @@`
		`# should only be used when configured for SHA512`
		`self.source_entry_type = 'bsd' if self.lookasidehash != 'md5' else 'old'`
		`self.branchre = 'c\d{1,}(s)?(tream)?\|master'`
		`+ self.lookaside_structure = None`
		`+`
		`+ def update(self, config):`
		`+ if self.lookaside_structure is None:`
		`+ self.lookaside_structure = config.get('lookaside_structure', 'hash')`
bstinson commented 2 years ago Is there any way for us to add a command line switch to override/force the lookaside structure? Not necessarily something we need to address now, but would be a good enhancement I think

		`@property`
		`def distgitdir(self):`
		`@@ -168,7 +173,8 @@`
		`self.lookaside,`
		`self.lookaside_cgi,`
		`self.repo_name,`
		`- self.branch_merge)`
		`+ self.branch_merge,`
		`+ structure=self.lookaside_structure)`

		`# redefined loaders`
		`def load_rpmdefines(self):`

src/centpkg/cli.py

file modified

		`@@ -35,6 +35,11 @@`

		`self.setup_centos_subparsers()`

		`+ def load_cmd(self):`
		`+ super(centpkgClient, self).load_cmd()`
		`+ cfg = self.config[self.get_name()]`
		`+ self._cmd.update(cfg)`
		`+`
		`def setup_centos_subparsers(self):`
		`self.register_do_fork()`
		`self.register_request_gated_side_tag()`

src/centpkg/lookaside.py

file modified

+28 -7

		`@@ -94,7 +94,7 @@`
		`return super(StreamLookasideCache, self).remote_file_exists(`
		`_name, filename, hashstr)`

		`- def upload(self, name, filename, hashstr, offline=False):`
		`+ def upload(self, name, filename, hashstr, offline=False, **kwargs):`
		`"""`
		`Uploads a file to lookaside cache.`

		`@@ -169,13 +169,32 @@`


		`class SIGLookasideCache(CGILookasideCache):`
		`- def __init__(self, hashtype, download_url, upload_url, name, branch):`
		`+ def __init__(self, hashtype, download_url, upload_url, name, branch, structure='hash'):`
		`super(SIGLookasideCache, self).__init__(`
		`hashtype, download_url, upload_url, client_cert="/home/bstinson/.centos.cert")`
		`+`
		`+ self.name = name`
		`self.branch = branch`
		`-`
		`- self.download_path = (`
		`- '%(name)s/%(branch)s/%(hash)s')`
		`+ self.structure = structure`
		`+`
		`+ @property`
		`+ def download_path(self):`
		`+ if self.structure == 'hash':`
		`+ return '%(name)s/%(filename)s/%(hashtype)s/%(hash)s'`
		`+ return '%(name)s/%(branch)s/%(hash)s'`
		`+`
		`+ def get_download_url(self, name, filename, hash, hashtype=None, **kwargs):`
		`+ if self.structure == 'hash':`
		`+ path_dict = {`
		`+ 'name': name,`
		`+ 'filename': filename,`
		`+ 'hash': hash,`
		`+ 'hashtype': hashtype`
		`+ }`
		`+ path = self.download_path % path_dict`
		`+ return os.path.join(self.download_url, path)`
		`+`
		`+ return super(SIGLookasideCache, self).get_download_url(name, filename, hash, hashtype, **kwargs)`

		`def remote_file_exists(self, name, filename, hash):`
		`"""Verify whether a file exists on the lookaside cache`
		`@@ -201,8 +220,9 @@`

		`post_data = [('name', _name),`
		`('%ssum' % self.hashtype, hash),`
		`- ('branch', self.branch),`
		`('filename', filename)]`
		`+ if self.structure == 'branch':`
		`+ post_data.append(('branch', self.branch))`

		`with io.BytesIO() as buf:`
		`c = pycurl.Curl()`
		`@@ -279,8 +299,9 @@`
		`self.log.info("Uploading: %s", filepath)`
		`post_data = [('name', name),`
		`('%ssum' % self.hashtype, hash),`
		`- ('branch', self.branch),`
		`('file', (pycurl.FORM_FILE, filepath))]`
		`+ if self.structure == 'branch':`
		`+ post_data.append(('branch', self.branch))`

		`with io.BytesIO() as buf:`
		`c = pycurl.Curl()`

lrossett commented 2 years ago

Adds support to the new lookaside cache structure
Adds new config file entry lookaside_structure to indicate which format to use, possible value are branch (current/legacy) and hash (new), defaults to hash
This change does not impact centpkg just centpkg-sig

I am still running tests but it is in a state where the code can be reviewed.

pingou commented on line 53 of src/centpkg/lookaside.py 2 years ago

typo here :)

pingou commented on line 18 of src/centpkg/__init__.py 2 years ago

Stupid nitpick: I've tend to use: config.get("key") or "default" as otherwise if key is defined to None in config, you will end up with None as value :)

pingou commented on line 8 of src/centpkg-sig.conf 2 years ago

Maybe add # lookaside_structure = branch as well and a comment explaining both?

lrossett commented 2 years ago

Stupid nitpick: I've tend to use: config.get("key") or "default" as otherwise if key is defined to None in config, you will end up with None as value :)

That is actually a dict, so it will return "hash" as default value instead of None.

I did it this way so the object property will only be updated once.

rebased onto 54932707f84ba0178f1e9152ab3ec06b0a064a6d

2 years ago

pingou commented on line 6 of src/centpkg/__init__.py 2 years ago

Do we need the second space here?

rebased onto f5ba265

2 years ago

bstinson commented on line 9 of src/centpkg/__init__.py 2 years ago

Is there any way for us to add a command line switch to override/force the lookaside structure? Not necessarily something we need to address now, but would be a good enhancement I think

bstinson commented 2 years ago

:thumbsup: ready to merge

lrossett commented 2 years ago

Thanks, I will merge this one and add the cli parameter in another pull request.

Pull-Request has been merged by lrossett

2 years ago