Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-1349

Change marshaling code to use json

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: Middleware
    • Labels:
      None

      Description

      The marshaling code for non-standard (i.e., non-filterable) components of messages is custom and not standard. Change this to use JSON.

        Attachments

          Issue Links

            Activity

            spietrowicz Steve Pietrowicz created issue -
            spietrowicz Steve Pietrowicz made changes -
            Field Original Value New Value
            Epic Link DM-1121 [ 13930 ]
            spietrowicz Steve Pietrowicz made changes -
            Summary Change marshaling code to use jscon Change marshaling code to use json
            spietrowicz Steve Pietrowicz made changes -
            Story Points 6 4
            spietrowicz Steve Pietrowicz made changes -
            Rank Ranked higher
            spietrowicz Steve Pietrowicz made changes -
            Sprint W15 Dec Sprint 1 [ 113 ]
            spietrowicz Steve Pietrowicz made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            mgelman2 Margaret Gelman made changes -
            Rank Ranked higher
            bglick Bill Glick made changes -
            Sprint W15 Dec Sprint 1 [ 113 ] W15 Dec Sprint 1, W15 Dec Sprint 2 [ 113, 116 ]
            bglick Bill Glick made changes -
            Rank Ranked lower
            spietrowicz Steve Pietrowicz made changes -
            Status In Progress [ 3 ] To Do [ 10001 ]
            Hide
            spietrowicz Steve Pietrowicz added a comment -

            Russell, would you be able to do a code review for me? It's two functions in one file in ctrl_events, in src/Event.cc

            ctrl_events/tickets/DM-1349

            https://dev.lsstcorp.org/cgit/LSST/DMS/ctrl_events.git/commit/?h=tickets/DM-1349&id=41bb9fa3178119dc94dff75c282c37b9b8869090

            Show
            spietrowicz Steve Pietrowicz added a comment - Russell, would you be able to do a code review for me? It's two functions in one file in ctrl_events, in src/Event.cc ctrl_events/tickets/ DM-1349 https://dev.lsstcorp.org/cgit/LSST/DMS/ctrl_events.git/commit/?h=tickets/DM-1349&id=41bb9fa3178119dc94dff75c282c37b9b8869090
            spietrowicz Steve Pietrowicz made changes -
            Reviewers Russell Owen [ rowen ]
            Status To Do [ 10001 ] In Review [ 10004 ]
            Hide
            rowen Russell Owen added a comment -

            This looks acceptable, but I suggest trying to figure out if there's a cleaner way to do this than long if/else statements with so much repetitive boilerplate. Admittedly that code already existed, but the use of boost has added even more lines of boilerplate to Event::marshall.

            One thought: construct a templated class that can marshal and unmarshal one data type. Then use a pair of maps (or unordered_map, now that we support C++11) to hold instances of these classes: one map that is used by marshall uses typeid(type) as keys. The other map is used by unmarshall and so has keys that are type names (strings).

            Show
            rowen Russell Owen added a comment - This looks acceptable, but I suggest trying to figure out if there's a cleaner way to do this than long if/else statements with so much repetitive boilerplate. Admittedly that code already existed, but the use of boost has added even more lines of boilerplate to Event::marshall. One thought: construct a templated class that can marshal and unmarshal one data type. Then use a pair of maps (or unordered_map, now that we support C++11) to hold instances of these classes: one map that is used by marshall uses typeid(type) as keys. The other map is used by unmarshall and so has keys that are type names (strings).
            rowen Russell Owen made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            spietrowicz Steve Pietrowicz made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            Hide
            spietrowicz Steve Pietrowicz added a comment -

            Thanks for the review.

            I ended up moving that common code to a templated method within Event.c. I also removed some now obsolete methods which are no longer in use. This reduced the common code and the size of the overall module.

            Show
            spietrowicz Steve Pietrowicz added a comment - Thanks for the review. I ended up moving that common code to a templated method within Event.c. I also removed some now obsolete methods which are no longer in use. This reduced the common code and the size of the overall module.
            spietrowicz Steve Pietrowicz made changes -
            Story Points 4 10
            spietrowicz Steve Pietrowicz made changes -
            Link This issue relates to DM-1350 [ DM-1350 ]
            frossie Frossie Economou made changes -
            Team Process Middleware [ 10206 ] Data Facility [ 12219 ]

              People

              • Assignee:
                spietrowicz Steve Pietrowicz
                Reporter:
                spietrowicz Steve Pietrowicz
                Reviewers:
                Russell Owen
                Watchers:
                Russell Owen, Steve Pietrowicz
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel