#641 [frontend] Pagure CI: allow more Package builds for one commit
Merged 7 months ago by praiskup. Opened 7 months ago by praiskup.
copr/ praiskup/copr fix-ci-rebuilds  into  master

@@ -119,13 +119,15 @@ 

                                             models.Package.name == package_name)

  

      @classmethod

-     def get_by_dir_name(cls, copr_dir_name, package_name):

+     def get_by_dir(cls, copr_dir, package_name):

          return models.Package.query.join(models.CoprDir).filter(

-             models.CoprDir.name == copr_dir_name, models.Package.name == package_name)

+             models.CoprDir.id==copr_dir.id,

+             models.Package.name==package_name

+         )

  

      @classmethod

      def get_or_create(cls, copr_dir, package_name, src_pkg):

-         package = cls.get_by_dir_name(copr_dir.name, package_name).first()

+         package = cls.get_by_dir(copr_dir, package_name).first()

  

          if package:

              return package

@@ -95,9 +95,7 @@ 

            {% if build.scm_object_type == 'pull-request' %}

            <dt> Pull Request: </dt>

            <dd>

-             <a href="{{ build.scm_object_url }}">

-               #{{ build.scm_object_id }}

-             </a>

+             PR#{{ build.scm_object_id }}

            </dd>

            {% endif %}

            <dt> Directory: </dt>

@@ -40,31 +40,35 @@ 

  

  log.info("ENDPOINT = {}".format(ENDPOINT))

  

- TOPICS = {

-     'io.pagure.prod.pagure.git.receive': 'https://pagure.io/',

-     'io.pagure.prod.pagure.pull-request.new': 'https://pagure.io/',

-     'io.pagure.prod.pagure.pull-request.comment.added': 'https://pagure.io/',

-     'org.fedoraproject.prod.pagure.git.receive': 'https://src.fedoraproject.org/',

-     'org.fedoraproject.prod.pagure.pull-request.new': 'https://src.fedoraproject.org/',

-     'org.fedoraproject.prod.pagure.pull-request.comment.added': 'https://src.fedoraproject.org/',

-     'io.pagure.stg.pagure.git.receive': 'https://stg.pagure.io/', # testing only

-     'io.pagure.stg.pagure.pull-request.new': 'https://stg.pagure.io/', # testing only

-     'io.pagure.stg.pagure.pull-request.comment.added': 'https://stg.pagure.io/', # testing only

+ pagure_instances = {

+     'https://pagure.io/':             'io.pagure.prod.pagure',

+     'https://src.fedoraproject.org/': 'org.fedoraproject.prod.pagure',

+     'https://stg.pagure.io/':         'io.pagure.stg.pagure', # testing only

  }

  

+ topics = [

+     'git.receive',

+     'pull-request.new',

+     'pull-request.rebased',

+     'pull-request.updated',

+     'pull-request.comment.added',

+ ]

+ 

+ TOPICS = {}

+ for url, fedmsg_prefix in pagure_instances.items():

+     for topic in topics:

+         TOPICS['{0}.{1}'.format(fedmsg_prefix, topic)] = url

+ 

  

  class ScmPackage(object):

      def __init__(self, db_row):

-         self.pkg_id = db_row.package_id

-         self.copr_id = db_row.copr_id

- 

          self.source_json_dict = json.loads(db_row.source_json)

          self.clone_url = self.source_json_dict.get('clone_url') or ''

          self.committish = self.source_json_dict.get('committish') or ''

          self.subdirectory = self.source_json_dict.get('subdirectory') or ''

  

-         self.package = ComplexLogic.get_package_by_id_safe(self.pkg_id)

-         self.copr = ComplexLogic.get_copr_by_id_safe(self.copr_id)

+         self.package = ComplexLogic.get_package_by_id_safe(db_row.package_id)

+         self.copr = self.package.copr

  

      def build(self, source_dict_update, copr_dir, update_callback,

                scm_object_type, scm_object_id, scm_object_url):

@@ -75,10 +79,6 @@ 

              package = self.package

  

          db.session.execute('LOCK TABLE build IN EXCLUSIVE MODE')

-         if models.Build.query.filter(models.Build.scm_object_url == scm_object_url).first():

-             log.info('\t -> Build for {} already exists.'.format(scm_object_url))

-             return None

- 

          return BuildsLogic.rebuild_package(

              package, source_dict_update, copr_dir, update_callback,

              scm_object_type, scm_object_id, scm_object_url)

@@ -116,7 +116,7 @@ 

          return False

  

  

- def event_info_from_pr_update(data, base_url):

+ def event_info_from_pr_comment(data, base_url):

      """

      Message handler for updated pull-request opened in pagure.

      Topic: ``*.pagure.pull-request.comment.added``

@@ -130,8 +130,12 @@ 

          return False

  

      last_comment = data['msg']['pullrequest']['comments'][-1]

-     if not last_comment or last_comment['notification'] is False:

-         log.info('Comment was not a notification, discarding.')

+     if not last_comment:

+         log.info('Can not access last comment, discarding.')

+         return False

+ 

+     if not 'comment' in last_comment or '[copr-build]' not in last_comment['comment']:

+         log.info('The [copr-build] is not present in the message.')

          return False

  

      return munch.Munch({

@@ -150,7 +154,7 @@ 

      })

  

  

- def event_info_from_new_pr(data, base_url):

+ def event_info_from_pr(data, base_url):

      """

      Message handler for new pull-request opened in pagure.

      Topic: ``*.pagure.pull-request.new``

@@ -233,10 +237,10 @@ 

              log.error('Unknown topic {} received. Continuing.')

              continue

  

-         if re.match(r'^.*.pull-request.new$', data['topic']):

-             event_info = event_info_from_new_pr(data, base_url)

+         if re.match(r'^.*.pull-request.(new|rebased|updated)$', data['topic']):

+             event_info = event_info_from_pr(data, base_url)

          elif re.match(r'^.*.pull-request.comment.added$', data['topic']):

-             event_info = event_info_from_pr_update(data, base_url)

+             event_info = event_info_from_pr_comment(data, base_url)

          else:

              event_info = event_info_from_push(data, base_url)

  

@@ -259,7 +263,13 @@ 

                  log.error('Bad http status {0} from url {1}'.format(r.status_code, raw_commit_url))

  

          for pkg in candidates:

-             log.info('Considering pkg id: {}, source_json: {}'.format(pkg.pkg_id, pkg.source_json_dict))

+             package = '{}/{}(id={})'.format(

+                     pkg.package.copr.full_name,

+                     pkg.package.name,

+                     pkg.package.id

+             )

+             log.info('Considering pkg package: {}, source_json: {}'

+                         .format(package, pkg.source_json_dict))

  

              if (git_compare_urls(pkg.clone_url, event_info.base_clone_url)

                      and (not pkg.committish or event_info.branch_to.endswith(pkg.committish))

@@ -271,8 +281,8 @@ 

                      dirname = pkg.copr.name + ':pr:' + str(event_info.object_id)

                      copr_dir = CoprDirsLogic.get_or_create(pkg.copr, dirname)

                      update_callback = 'pagure_flag_pull_request'

-                     scm_object_url = os.path.join(base_url, event_info.base_project_url_path,

-                                                   'pull-request', str(event_info.object_id))

+                     scm_object_url = os.path.join(base_url, event_info.project_url_path,

+                                                   'c', str(event_info.end_commit))

                  else:

                      copr_dir = pkg.copr.main_dir

                      update_callback = 'pagure_flag_commit'

Previously, if even only one build existed in database for
particular commit hash in pagure (regardless of the package or
copr project), the pagure-events.py did not start the build.

The most frequent problem would be in forked projects probably;
project forks inherit packages with theirs SCM configuration and
then automatically one pull-requests means multiple builds. The
previous code though built only into one project (randomly).

Fixes: #586

I installed this as hot-fix on staging... and it seems to start the build:
https://copr-fe-dev.cloud.fedoraproject.org/coprs/g/copr/copr-dev/build/839642/

Let me try a rebase :-) I want to make the message nicer anyways.

rebased onto 9e1d0d16244e4af1afd95dc9fe06818ca71f6e2f

7 months ago

I removed the lock by accident, and there's at least one other bug for the pull-request case.

1 new commit added

  • fix
7 months ago

1 new commit added

  • Another fix.
7 months ago

1 new commit added

  • fixup UI, pointing at PR would require more changes
7 months ago

rebased onto daba405eb4890aa0acad44a677b4fd4a6a4549fb

7 months ago

2 new commits added

  • [frontend] better parse Pagure's PR messages
  • [frontend] allow more Package builds for one commit
7 months ago

2 new commits added

  • [frontend] better parse Pagure's PR messages
  • [frontend] allow more Package builds for one commit
7 months ago

2 new commits added

  • [frontend] better parse Pagure's PR messages
  • [frontend] allow more Package builds for one commit
7 months ago

1 new commit added

  • ugly hacks... :-(
7 months ago

3 new commits added

  • ugly hacks... :-(
  • [frontend] better parse Pagure's PR messages
  • [frontend] allow more Package builds for one commit
7 months ago

rebased onto bd5106d7b5d1efb5bbaa2a455b023cb87297568e

7 months ago

rebased onto 08666f316677069c3c8a7a710ceda3cfc6323d91

7 months ago

Metadata Update from @praiskup:
- Pull-request tagged with: needs-work

7 months ago

rebased onto b96057657116892b7a6c9bdc16bced1ba9f4bcd2

7 months ago

2 new commits added

  • [frontend] better parse Pagure's PR messages
  • [frontend] allow more Package builds for one commit
7 months ago

2 new commits added

  • [frontend] better parse Pagure's PR messages
  • [frontend] allow more Package builds for one commit
7 months ago

2 new commits added

  • [frontend] better parse Pagure's PR messages
  • [frontend] allow more Package builds for one commit
7 months ago

2 new commits added

  • [frontend] better parse Pagure's PR messages
  • [frontend] allow more Package builds for one commit
7 months ago

rebased onto 6364896183d4f171bda4820c86d51480d0a85680

7 months ago

rebased onto 2df847ee9a950e22a4d3623340ce1508bced9e26

7 months ago

Metadata Update from @praiskup:
- Pull-request untagged with: needs-work

7 months ago

Uhm, finally :-( this was unbelievably tough one, mostly because design issue in #617.

Can anyone take a look please?

I am mostly thinking about removing the "optimization" for max one build per one scm_object_url. It would be very useful to have [copr-test] keyword in Pagure to be able to trigger rebuild manually (because of temporary issues in automation, e.g.). WDYT? I would be able to properly reference Pagure PR web-ui in Copr UI, and it should bring zero risk.

Metadata Update from @praiskup:
- Pull-request tagged with: review

7 months ago

rebased onto 50bb396449e8edfb49a88320e40b226ef83533e1

7 months ago

rebased onto f60b858cd136509724778df746fcada26e282d7b

7 months ago

I don't see that much into webhooks code, but the changes look reasonable.
+1 from me

After irc chat with Jakub: I'll try to remove the "one-build-for-one-scm_object_url" protector here in this PR first.

Metadata Update from @praiskup:
- Pull-request untagged with: review
- Pull-request tagged with: needs-work

7 months ago

FWIW, if pull-request (or a push even) contains multiple commits, only the last commit is analyzed and only those packages which are touched by the last commit are rebuilt (for more info, see def is_dir_in_commit(). I reported an RFE against Pagure for this purpose, but this PR doesn't try to solve that particular problem.

rebased onto f237539

7 months ago

Ok, I removed the "duplicate build" detection, and added a support for [copr-build] keyword in comment, the CI functionality can be tested here:
https://stg.pagure.io/copr-pagure-events-testing/copr

Metadata Update from @praiskup:
- Pull-request untagged with: needs-work
- Pull-request tagged with: review

7 months ago

Pull-Request has been merged by praiskup

7 months ago