PR#169: Implementation of `GenericBuilder.repo_from_tag` for copr backend - fm-orchestrator

		`@@ -32,7 +32,7 @@`
		`from module_build_service.config import Config`
		`from module_build_service.pdc import (`
		`get_pdc_client_session, get_module, get_module_runtime_dependencies,`
		`- get_module_tag, get_module_build_dependencies, get_module_repo)`
		`+ get_module_tag, get_module_build_dependencies)`
		`import module_build_service.auth`


		`@@ -86,10 +86,6 @@`
		`print ("build_deps=%s" % get_module_build_dependencies(`
		`pdc_session, module))`
		`print ("tag=%s" % get_module_tag(pdc_session, module))`
		`-`
		`- module2 = get_module(pdc_session, {"name": "coprtestmodule", "version": "4.3.43", "release": 1})`
		`- print ("pdc_data=%s" % str(module))`
		`- print ("repo=%s" % get_module_repo(pdc_session, module2))`
		`else:`
		`print ('module was not found')`

module_build_service/builder.py

file modified

+25

		`@@ -182,6 +182,8 @@`
		`"""`
		`if backend == "koji":`
		`return KojiModuleBuilder.repo_from_tag(config, tag_name, arch)`
		`+ if backend == "copr":`
		`+ return CoprModuleBuilder.repo_from_tag(config, tag_name, arch)`
		`else:`
		`raise ValueError("Builder backend='%s' not recognized" % backend)`

		`@@ -845,3 +847,26 @@`
		`def get_disttag_srpm(disttag):`
		`# @FIXME`
		`return KojiModuleBuilder.get_disttag_srpm(disttag)`
		`+`
		`+ @classmethod`
		`+ def repo_from_tag(cls, config, tag_name, arch):`
		`+ """`
		`+ :param backend: a string representing the backend e.g. 'koji'.`
		`+ :param config: instance of rida.config.Config`
		`+ :param tag_name: Tag for which the repository is returned`
		`+ :param arch: Architecture for which the repository is returned`
		`+`
		`+ Returns URL of repository containing the built artifacts for`
		`+ the tag with particular name and architecture.`
		`+ """`
		`+ from copr.client import CoprClient`
		`+`
		`+ # @TODO get the correct user`
		`+ # Premise is that tag_name is in name-version-release format`
		`+ owner, nvr = "@copr", tag_name`
		`+ cl = CoprClient.create_from_file_config(config.copr_config)`
		`+ response = cl.get_module_repo(owner, nvr).data`
		`+`
		`+ if response["output"] == "notok":`
		`+ raise ValueError(response["error"])`
		`+ return response["repo"]`

module_build_service/pdc.py

file modified

-32

		`@@ -157,38 +157,6 @@`
		`"""`
		`return get_module(session, module_info, strict=strict)['koji_tag']`

		`- def get_module_repo(session, module_info, strict=False, config=module_build_service.conf):`
		`- """`
		`- :param session : PDCClient instance`
		`- :param module_info: list of module_info dicts`
		`- :param strict: Normally this function returns None if no module can be`
		`- found. If strict=True, then a ValueError is raised.`
		`- :param config: instance of module_build_service.config.Config`
		`- :return: URL to a DNF repository for the module`
		`- """`
		`- module = get_module(session, module_info, strict=strict)`
		`- if not module:`
		`- return None`
		`-`
		`- # Module was built in Koji`
		`- # @TODO There should be implemented retrieveing URL to a module repofile in koji`
		`- if module["koji_tag"] != "-":`
		`- raise NotImplementedError`
		`-`
		`- # TODO We should revisit the decision to include CoprClient code in the`
		`- # pdc.py module.`
		`- from copr.client import CoprClient`
		`-`
		`- # Module was built in Copr`
		`- # @TODO get the correct user`
		`- owner, nvr = "@copr", module["variant_id"]`
		`- cl = CoprClient.create_from_file_config(config.copr_config)`
		`- response = cl.get_module_repo(owner, nvr).data`
		`-`
		`- if response["output"] == "notok":`
		`- raise ValueError(response["error"])`
		`- return response["repo"]`
		`-`
		`def module_depsolving_wrapper(session, module_list, strict=True):`
		`"""`
		`:param session : PDCClient instance`

tests/test_copr.py

file added

+65

		`@@ -0,0 +1,65 @@`
		`+ # Copyright (c) 2016 Red Hat, Inc.`
		`+ #`
		`+ # Permission is hereby granted, free of charge, to any person obtaining a copy`
		`+ # of this software and associated documentation files (the "Software"), to deal`
		`+ # in the Software without restriction, including without limitation the rights`
		`+ # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell`
		`+ # copies of the Software, and to permit persons to whom the Software is`
		`+ # furnished to do so, subject to the following conditions:`
		`+ #`
		`+ # The above copyright notice and this permission notice shall be included in all`
		`+ # copies or substantial portions of the Software.`
		`+ #`
		`+ # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR`
		`+ # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,`
		`+ # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE`
		`+ # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER`
		`+ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,`
		`+ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
		`+ # SOFTWARE.`
		`+ #`
		`+ # Written by Jakub Kadlcik <jkadlcik@redhat.com>`
		`+`
		`+`
		`+ import unittest`
		`+ import mock`
		`+ import module_build_service.builder`
		`+`
		`+`
		`+ @unittest.skip("We need not yet released version of python-copr. Let's skip this for some time")`
		`+ class TestCoprBuilder(unittest.TestCase):`
		`+`
		`+ def setUp(self):`
		`+ self.config = mock.Mock()`
		`+ self.config.copr_config = None`
		`+`
		`+ @mock.patch("copr.CoprClient.get_module_repo")`
		`+ def test_tag_to_repo(self, get_module_repo):`
		`+ # Mock the CoprClient.get_module_repo to return something, without requesting a Copr instance`
		`+ def get_module_repo_mock(owner, nvr):`
		`+ return ResponseMock({`
		`+ "output": "ok",`
		`+ "repo": "http://copr-be-instance/results/{}/{}/modules".format(owner, nvr)`
		`+ })`
		`+ get_module_repo.side_effect = get_module_repo_mock`
		`+`
		`+ repo = module_build_service.builder.GenericBuilder.tag_to_repo(`
		`+ "copr", self.config, "foo-module-name-0.25-9", None)`
		`+ self.assertEquals(repo, "http://copr-be-instance/results/@copr/foo-module-name-0.25-9/modules")`
		`+`
		`+ @mock.patch("copr.CoprClient.get_module_repo")`
		`+ def test_non_existing_tag_to_repo(self, get_module_repo):`
		`+ # Let's pretend that CoprClient.get_module_repo couldn't find the project on Copr instance`
		`+ get_module_repo.return_value = ResponseMock({"output": "notok", "error": "some error"})`
		`+ self.assertRaises(ValueError,`
		`+ lambda: module_build_service.builder.GenericBuilder.tag_to_repo(`
		`+ "copr", self.config, None, None))`
		`+`
		`+`
		`+ class ResponseMock(object):`
		`+ def __init__(self, data):`
		`+ self._data = data`
		`+`
		`+ @property`
		`+ def data(self):`
		`+ return self._data`

frostyx commented 7 years ago

Since my last PR#103 there was added repo_from_tag method for builders. This PR adds implementation of the method for "copr" backend and drops the redundant get_module_repo method.

1 new commit added

Add missing docstring

7 years ago

mprahl commented on line 35 of tests/test_copr.py 7 years ago

This should be /etc/module_build_service/copr.conf since the service was renamed.

mprahl commented 7 years ago

@frostyx can you try rebasing to the latest master to see if this fixes the unit tests?

ralph commented 7 years ago

Looks like that won't do it @mprahl:

(rida) ❯ git rebase master            fm-orchestrator/git/copr-repo_from_tag 
Current branch copr-repo_from_tag is up to date.

I think the tests are genuinely failing. Needs some work still from @frostyx.

mprahl commented on line 28 of tests/test_copr.py 7 years ago

I think this is why the tests are failing. Use:
from copr import CoprClient

mprahl commented 7 years ago

@ralph thanks for pointing that out. I found the issue. It is an incorrect import statement.

frostyx commented on line 28 of tests/test_copr.py 7 years ago

Are you sure? It seems to be valid import for me.
I would guess that copr python package is not installed, but it is listed in test-requirements.txt, so I don't know.

1 new commit added

Value of copr_config is not relevant for this tests

7 years ago

mprahl commented 7 years ago

@frostyx the import I provided works for me:

[vagrant@localhost src]$ python
Python 2.7.12 (default, Sep 29 2016, 13:30:34) 
[GCC 6.2.1 20160916 (Red Hat 6.2.1-2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from copr import CoprClient
>>> CoprClient
<class 'copr.client.client.CoprClient'>

frostyx commented 7 years ago

@frostyx the import I provided works for me:

Well yes, but this too

[vagrant@localhost vagrant]$ python
>>> from copr.client import CoprClient
>>> CoprClient
<class 'copr.client.client.CoprClient'>

That is why I doubt that it will help.

mprahl commented 7 years ago

@frostyx there are new errors in the unit tests. Please correct them.

rebased

7 years ago

1 new commit added

Change CoprClient import

7 years ago

1 new commit added

Skip copr tests

7 years ago

6 new commits added

Patch tests without having to import CoprClient
Skip copr tests
Value of copr_config is not relevant for this tests
Add missing docstring
Drop get_module_repo function since it is in GenericBuilder
Implement GenericBuilder.repo_from_tag for copr backend

7 years ago

frostyx commented 7 years ago

This PR (well, also the already merged code) assumes still unreleased version of python-copr. I suggest to skip copr tests until new python-copr is released.

Those other tests what fails in this moment, .... is it caused by this PR? I've rebased from the master, but it haven't fixed anything.

ralph commented 7 years ago

Ah, those breakages look like they are the results of the latest modulemd release.

ralph commented 7 years ago

I verified this locally. Good to merge!

ralph commented 7 years ago

(done now, this is in master)