#370 Add new 'metadata' field in the manual rebuild API to store additional metadata for Event.
Merged 5 months ago by jkaluza. Opened 5 months ago by jkaluza.
jkaluza/freshmaker requester_metadata  into  master

file modified
+3 -1

@@ -316,7 +316,8 @@ 

      from advisory.

      """

  

-     def __init__(self, msg_id, advisory, container_images, **kwargs):

+     def __init__(self, msg_id, advisory, container_images,

+                  requester_metadata_json=None, **kwargs):

          """

          Creates new ManualRebuildWithAdvisoryEvent.

  

@@ -328,6 +329,7 @@ 

          super(ManualRebuildWithAdvisoryEvent, self).__init__(

              msg_id, advisory, **kwargs)

          self.container_images = container_images

+         self.requester_metadata_json = requester_metadata_json

Do we need to validate that this is an JSON object? Instead, of say an Array?

We validate it by json.dumps() in the views.py. The json.dumps() would fail in case you specify list there. I will write a test-case for that.

Actually, whole request data must be a json and therefore the metadata part of this is also JSON, so this should be OK.

  

  

  class BrewSignRPMEvent(BaseEvent):

@@ -0,0 +1,22 @@ 

+ """Add requester_metadata to Events table.

+ 

+ Revision ID: 5bdd5566615a

+ Revises: 5a555923da42

+ Create Date: 2019-02-25 15:02:13.847086

+ 

+ """

+ 

+ # revision identifiers, used by Alembic.

+ revision = '5bdd5566615a'

+ down_revision = '5a555923da42'

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ def upgrade():

+     op.add_column('events', sa.Column('requester_metadata', sa.String(), nullable=True))

Could we possibly use PostgreSQL's json support? Not sure how well that will play with SQLAlchemy

This is not not well supported in SQLAlchemy and brings no real benefits so far. We do not need to query that. We can always convert to JSON later by migration script if there will ever be need to do some more complex transformations.

+ 

+ 

+ def downgrade():

+     op.drop_column('events', 'requester_metadata')

file modified
+10

@@ -155,6 +155,9 @@ 

      # (for example NVR of container images) to rebuild if passed using the

      # REST API.

      requested_rebuilds = db.Column(db.String, nullable=True)

+     # For manual rebuilds, contains the serialized JSON optionally submitted

+     # by the requester to track the context of this event.

+     requester_metadata = db.Column(db.String, nullable=True)

  

      manual_triggered = db.Column(

          db.Boolean,

@@ -332,6 +335,12 @@ 

              type_name = "UnknownEventType %d" % self.event_type_id

          return "<%s, search_key=%s>" % (type_name, self.search_key)

  

+     @property

+     def requester_metadata_json(self):

+         if not self.requester_metadata:

+             return {}

+         return json.loads(self.requester_metadata)

+ 

      def json(self):

          data = self._common_json()

          data['builds'] = [b.json() for b in self.builds]

@@ -364,6 +373,7 @@ 

              "requester": self.requester,

              "requested_rebuilds": (self.requested_rebuilds.split(" ")

                                     if self.requested_rebuilds else []),

+             "requester_metadata": self.requester_metadata_json,

          }

  

      def find_dependent_events(self):

@@ -51,8 +51,8 @@ 

          advisory = ErrataAdvisory.from_advisory_id(errata, errata_id)

  

          event = ManualRebuildWithAdvisoryEvent(

-             msg_id, advisory, data.get("container_images", []), manual=True,

-             dry_run=dry_run)

+             msg_id, advisory, data.get("container_images", []), data.get("metadata", None),

+             manual=True, dry_run=dry_run)

  

          return event

  

file modified
+3

@@ -21,6 +21,7 @@ 

  #

  # Written by Jan Kaluza <jkaluza@redhat.com>

  

+ import json

  import six

  from flask import request, jsonify

  from flask.views import MethodView

@@ -285,6 +286,8 @@ 

          db_event = models.Event.get_or_create_from_event(db.session, event)

          db_event.requester = g.user.username

          db_event.requested_rebuilds = " ".join(event.container_images)

+         if event.requester_metadata_json:

+             db_event.requester_metadata = json.dumps(event.requester_metadata_json)

          db.session.commit()

  

          # Forward the POST data (including the msg_id of the database event we

file modified
+1

@@ -184,6 +184,7 @@ 

              'state_reason': None,

              'url': 'http://localhost:5001/api/1/events/1',

              'requested_rebuilds': [],

+             'requester_metadata': {},

          })

  

  

file modified
+24 -1

@@ -507,7 +507,8 @@ 

              u'url': u'/api/1/events/1',

              u'dry_run': False,

              u'requester': 'tester1',

-             u'requested_rebuilds': []})

+             u'requested_rebuilds': [],

+             u'requester_metadata': {}})

          publish.assert_called_once_with(

              'manual.rebuild',

              {'msg_id': 'manual_rebuild_123', u'errata_id': 1})

@@ -554,6 +555,28 @@ 

              {'msg_id': 'manual_rebuild_123', u'errata_id': 1,

               'container_images': ["foo-1-1", "bar-1-1"]})

  

+     @patch('freshmaker.messaging.publish')

+     @patch('freshmaker.parsers.internal.manual_rebuild.ErrataAdvisory.'

+            'from_advisory_id')

+     @patch('freshmaker.parsers.internal.manual_rebuild.time.time')

+     def test_manual_rebuild_metadata(self, time, from_advisory_id, publish):

+         time.return_value = 123

+         from_advisory_id.return_value = ErrataAdvisory(

+             123, 'name', 'REL_PREP', ['rpm'])

+ 

+         resp = self.client.post(

+             '/api/1/builds/', data=json.dumps({

+                 'errata_id': 1, 'metadata': {"foo": ["bar"]}}),

+             content_type='application/json')

+         data = json.loads(resp.get_data(as_text=True))

+ 

+         # Other fields are predictible.

+         self.assertEqual(data['requester_metadata'], {"foo": ["bar"]})

+         publish.assert_called_once_with(

+             'manual.rebuild',

+             {'msg_id': 'manual_rebuild_123', u'errata_id': 1,

+              'metadata': {"foo": ["bar"]}})

+ 

  

  class TestOpenIDCLogin(ViewBaseTest):

      """Test that OpenIDC login"""

The 'metadata' can be set to any JSON and its value is availabe in the Events API
and also in the fedmsgs as 'requester_metadata'.

Could we possibly use PostgreSQL's json support? Not sure how well that will play with SQLAlchemy

Do we need to validate that this is an JSON object? Instead, of say an Array?

This is not not well supported in SQLAlchemy and brings no real benefits so far. We do not need to query that. We can always convert to JSON later by migration script if there will ever be need to do some more complex transformations.

We validate it by json.dumps() in the views.py. The json.dumps() would fail in case you specify list there. I will write a test-case for that.

Actually, whole request data must be a json and therefore the metadata part of this is also JSON, so this should be OK.

Pull-Request has been merged by jkaluza

5 months ago