#2051 hub: editTagExternalRepo is able to set merge_mode
Merged 4 months ago by tkopecek. Opened 5 months ago by julian8628.
julian8628/koji issue/1857  into  master

file modified
+32 -6

@@ -5470,22 +5470,48 @@ 

  

  def handle_edit_external_repo(goptions, session, args):

      "[admin] Edit data for an external repo"

-     usage = _("usage: %prog edit-external-repo <name>")

+     usage = _("usage: %prog edit-external-repo [options] <name>")

      parser = OptionParser(usage=get_usage_str(usage))

      parser.add_option("--url", help=_("Change the url"))

      parser.add_option("--name", help=_("Change the name"))

+     parser.add_option("-t", "--tag", metavar="TAG",

+                       help=_("Edit the repo properties for the tag."))

+     parser.add_option("-p", "--priority", metavar="PRIORITY", type='int',

+                       help=_("Edit the priority of the repo for the tag specified by --tag."))

+     parser.add_option("-m", "--mode", metavar="MODE",

+                       help=_("Edit the merge mode of the repo for the tag specified by --tag. "

+                              "Options: %s.") % ", ".join(koji.REPO_MERGE_MODES))

      (options, args) = parser.parse_args(args)

      if len(args) != 1:

          parser.error(_("Incorrect number of arguments"))

-     opts = {}

+     repo_opts = {}

      if options.url:

-         opts['url'] = options.url

+         repo_opts['url'] = options.url

      if options.name:

-         opts['name'] = options.name

-     if not opts:

+         repo_opts['name'] = options.name

+     tag_repo_opts = {}

+     if options.tag:

+         if options.priority is not None:

+             tag_repo_opts['priority'] = options.priority

+         if options.mode:

+             tag_repo_opts['merge_mode'] = options.mode

+         if not tag_repo_opts:

+             parser.error(_("At least, one of priority and merge mode should be specified"))

+         tag_repo_opts['tag_info'] = options.tag

+         tag_repo_opts['repo_info'] = args[0]

+     else:

+         for k in ('priority', 'mode'):

+             if getattr(options, k) is not None:

+                 parser.error(_("If %s is specified, --tag must be specified as well") % k)

+ 

+     if not (repo_opts or tag_repo_opts):

          parser.error(_("No changes specified"))

+ 

      activate_session(session, goptions)

-     session.editExternalRepo(args[0], **opts)

+     if repo_opts:

+         session.editExternalRepo(args[0], **repo_opts)

+     if tag_repo_opts:

+         session.editTagExternalRepo(**tag_repo_opts)

  

  

  def handle_remove_external_repo(goptions, session, args):

@@ -0,0 +1,11 @@ 

+ -- upgrade script to migrate the Koji database schema

+ -- from version 1.20 to 1.21

+ 

+ 

+ BEGIN;

+ 

+ -- merge_mode can not be null

+ UPDATE tag_external_repos SET merge_mode = 'koji' WHERE merge_mode is NULL;

+ ALTER TABLE tag_external_repos ALTER COLUMN merge_mode SET NOT NULL;

+ 

+ COMMIT;

file modified
+1 -1

@@ -484,7 +484,7 @@ 

  	tag_id INTEGER NOT NULL REFERENCES tag(id),

  	external_repo_id INTEGER NOT NULL REFERENCES external_repo(id),

  	priority INTEGER NOT NULL,

- 	merge_mode TEXT DEFAULT 'koji',

+ 	merge_mode TEXT NOT NULL DEFAULT 'koji',

  -- versioned - see earlier description of versioning

  	create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(),

  	revoke_event INTEGER REFERENCES events(id),

file modified
+21 -4

@@ -3667,6 +3667,9 @@ 

  

      context.session.assertPerm('tag')

  

+     # sanity check for None value, which may happen if DB schema isn't updated to 1.21+

+     if merge_mode is None:

+         merge_mode = 'koji'

      if merge_mode not in koji.REPO_MERGE_MODES:

          raise koji.GenericError('Invalid merge mode: %s' % merge_mode)

  

@@ -3710,9 +3713,12 @@ 

      update.execute()

  

  

- def edit_tag_external_repo(tag_info, repo_info, priority):

+ def edit_tag_external_repo(tag_info, repo_info, priority=None, merge_mode=None):

We should check, that merge_mode has correct value.

      """Edit a tag<->external repo association

-     This allows you to update the priority without removing/adding the repo."""

+     This allows you to update the priority and merge_mode without removing/adding the repo.

+ 

+     Note that None value of priority and merge_mode means no change on it

+     """

  

      context.session.assertPerm('tag')

  

@@ -3727,9 +3733,20 @@ 

                                  (repo['name'], tag['name']))

      tag_repo = tag_repos[0]

  

-     if priority != tag_repo['priority']:

+     data = {}

+     for k in ('priority', 'merge_mode'):

+         val = locals().get(k)

+         # None value means no change

+         if val is not None and val != tag_repo[k]:

+             data[k] = val

+     if not data:

+         return False

+     else:

+         for k in ('priority', 'merge_mode'):

+             data.setdefault(k, tag_repo[k])

          remove_external_repo_from_tag(tag_id, repo_id)

-         add_external_repo_to_tag(tag_id, repo_id, priority)

+         add_external_repo_to_tag(tag_id, repo_id, **data)

+         return True

  

  

  def get_tag_external_repos(tag_info=None, repo_info=None, event=None):

@@ -0,0 +1,101 @@ 

+ # coding=utf-8

+ from __future__ import absolute_import

+ import mock

+ import six

+ try:

+     import unittest2 as unittest

+ except ImportError:

+     import unittest

+ 

+ import koji

+ from koji_cli.commands import handle_edit_external_repo

+ from . import utils

+ 

+ 

+ class TestEditExternalRepo(utils.CliTestCase):

+ 

+     # Show long diffs in error output...

+     maxDiff = None

+ 

+     def setUp(self):

+         self.options = mock.MagicMock()

+         self.session = mock.MagicMock()

+         self.error_format = """Usage: %s edit-external-repo [options] <name>

+ (Specify the --help global option for a list of other help options)

+ 

+ %s: error: {message}

+ """ % (self.progname, self.progname)

+ 

+     @mock.patch('sys.stdout', new_callable=six.StringIO)

+     @mock.patch('sys.stderr', new_callable=six.StringIO)

+     @mock.patch('koji_cli.commands.activate_session')

+     def test_handle_edit_external_repo_error(

+             self,

+             activate_session_mock,

+             stderr,

+             stdout):

+         """Test handle_edit_external_repo function"""

+         # [(expected, args),...]

+         items = [

+             ("Incorrect number of arguments", []),

+             ("Incorrect number of arguments", ['arg1', 'arg2']),

+             ("No changes specified", ['ext_repo']),

+             ("At least, one of priority and merge mode should be specified",

+              ['ext_repo', '-t', 'tag']),

+             ("If priority is specified, --tag must be specified as well",

+              ['ext_repo', '-p', '0']),

+             ("If mode is specified, --tag must be specified as well",

+              ['ext_repo', '-m', 'koji'])

+ 

+         ]

+         for expected, cmd_args in items:

+             self.assert_system_exit(

+                 handle_edit_external_repo,

+                 self.options,

+                 self.session,

+                 cmd_args,

+                 stderr=self.format_error_message(expected),

+                 activate_session=None)

+ 

+         # edit ext-repo only

+         handle_edit_external_repo(self.options, self.session,

+                                   ['ext_repo','--name', 'newname', '--url', 'https://newurl'])

+         self.assert_console_message(stdout, "")

+         self.assert_console_message(stderr, "")

+         self.session.editExternalRepo.assert_called_once_with('ext_repo',

+                                                               name='newname', url='https://newurl')

+         self.session.editTagExternalRepo.assert_not_called()

+ 

+         # edit tag-repo only

+         self.session.reset_mock()

+         handle_edit_external_repo(self.options, self.session,

+                                   ['ext_repo','-t', 'tag', '-p', '0', '-m', 'koji'])

+         self.assert_console_message(stdout, "")

+         self.assert_console_message(stderr, "")

+         self.session.editExternalRepo.assert_not_called()

+         self.session.editTagExternalRepo.assert_called_once_with(repo_info='ext_repo',

+                                                                  tag_info='tag',

+                                                                  priority=0,

+                                                                  merge_mode='koji')

+ 

+     def test_handle_edit_external_repo_help(self):

+         self.assert_help(

+             handle_edit_external_repo,

+             """Usage: %s edit-external-repo [options] <name>

+ (Specify the --help global option for a list of other help options)

+ 

+ Options:

+   -h, --help            show this help message and exit

+   --url=URL             Change the url

+   --name=NAME           Change the name

+   -t TAG, --tag=TAG     Edit the repo properties for the tag.

+   -p PRIORITY, --priority=PRIORITY

+                         Edit the priority of the repo for the tag specified by

+                         --tag.

+   -m MODE, --mode=MODE  Edit the merge mode of the repo for the tag specified

+                         by --tag. Options: %s.

+ """ % (self.progname, ', '.join(koji.REPO_MERGE_MODES)))

+ 

+ 

+ if __name__ == '__main__':

+     unittest.main()

@@ -0,0 +1,85 @@ 

+ import mock

+ try:

+     import unittest2 as unittest

+ except ImportError:

+     import unittest

+ 

+ import koji

+ import kojihub

+ 

+ 

+ class TestEditTagExternalRepo(unittest.TestCase):

+ 

+     def setUp(self):

+         self.context = mock.patch('kojihub.context').start()

+         self.context.session.assertPerm = mock.MagicMock()

+         self.get_tag = mock.patch('kojihub.get_tag').start()

+         self.get_external_repo = mock.patch('kojihub.get_external_repo').start()

+         self.get_tag_external_repos = mock.patch('kojihub.get_tag_external_repos').start()

+         self.get_tag.return_value = {'id': 1, 'name': 'tag'}

+         self.get_external_repo.return_value = {'id': 11, 'name': 'ext_repo'}

+         self.get_tag_external_repos.return_value = [{'external_repo_id': 11,

+                                                      'tag_id': 1,

+                                                      'priority': 5,

+                                                      'merge_mode': 'simple'}]

+ 

+         self.remove_external_repo_from_tag = mock.patch(

+             'kojihub.remove_external_repo_from_tag').start()

+         self.add_external_repo_to_tag = mock.patch('kojihub.add_external_repo_to_tag').start()

+ 

+     def tearDown(self):

+         mock.patch.stopall()

+ 

+     def test_edit(self):

+         rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=6, merge_mode='bare')

+         self.get_tag.assert_called_once_with('tag', strict=True)

+         self.get_external_repo.assert_called_once_with('ext_repo', strict=True)

+         self.get_tag_external_repos.assert_called_once_with(tag_info=1, repo_info=11)

+         self.remove_external_repo_from_tag.assert_called_once_with(1, 11)

+         self.add_external_repo_to_tag.assert_called_once_with(1, 11, priority=6, merge_mode='bare')

+         self.assertTrue(rv)

+ 

+     def test_edit_no_tag_repo(self):

+         self.get_tag_external_repos.return_value = []

+         with self.assertRaises(koji.GenericError) as cm:

+             kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=6, merge_mode='bare')

+         self.assertEqual(cm.exception.args[0],

+                          'external repo ext_repo not associated with tag tag')

+         self.get_tag.assert_called_once_with('tag', strict=True)

+         self.get_external_repo.assert_called_once_with('ext_repo', strict=True)

+         self.get_tag_external_repos.assert_called_once_with(tag_info=1, repo_info=11)

+         self.remove_external_repo_from_tag.assert_not_called()

+         self.add_external_repo_to_tag.assert_not_called()

+ 

+     def test_edit_no_changes_2(self):

+         rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=5, merge_mode='simple')

+         self.remove_external_repo_from_tag.assert_not_called()

+         self.add_external_repo_to_tag.assert_not_called()

+         self.assertFalse(rv)

+ 

+     def test_edit_all_none(self):

+         self.get_tag_external_repos.return_value = [{'external_repo_id': 11,

+                                                      'tag_id': 1,

+                                                      'priority': None,

+                                                      'merge_mode': None}]

+         rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=None, merge_mode=None)

+         self.remove_external_repo_from_tag.assert_not_called()

+         self.add_external_repo_to_tag.assert_not_called()

+         self.assertFalse(rv)

+ 

+     def test_edit_none_new(self):

+         rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=None, merge_mode=None)

+         self.remove_external_repo_from_tag.assert_not_called()

+         self.add_external_repo_to_tag.assert_not_called()

+         self.assertFalse(rv)

+ 

+     def test_edit_none_old(self):

+         self.get_tag_external_repos.return_value = [{'external_repo_id': 11,

+                                                      'tag_id': 1,

+                                                      'priority': 5,

+                                                      'merge_mode': None}]

+         rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=None, merge_mode='simple')

+         self.remove_external_repo_from_tag.assert_called_once_with(1, 11)

+         self.add_external_repo_to_tag.assert_called_once_with(1, 11,

+                                                               priority=5, merge_mode='simple')

+         self.assertTrue(rv)

WIP for adding a new ability to edit-external-repo cmd, so that this cmd can edit priority and merge_mode for a tag -- external-repo config

done

rebased onto e6b557b35ceb996ada8e629db45a6cffcac4a70f

4 months ago

rebased onto f082f1c5fb7c41ebe6d41b3a6d39e06d352ea9ed

4 months ago

3 new commits added

  • unittests: for edit-external-repo cmd and editTagExternalRepo API
  • cli: extend edit-external-repo to support change of priority and merge mode
  • hub: editTagExternalRepo is able to set merge_mode
4 months ago

rebased onto 4513f1e

4 months ago

We should check, that merge_mode has correct value.

We should check, that merge_mode has correct value.

It's checked in add_external_repo_to_tag, so I don't do it in edit_tag_external_repo, as well as cli cmd

Ah, you're right, it works.

:thumbsup:

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

4 months ago

pretty please pagure-ci rebuild

4 months ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done

4 months ago

Commit ae8958e fixes this pull-request

Pull-Request has been merged by tkopecek

4 months ago